[Cases] Fix unified attachment gaps when attachments.enabled is true#257767
Closed
patrykkopycinski wants to merge 2 commits into
Closed
[Cases] Fix unified attachment gaps when attachments.enabled is true#257767patrykkopycinski wants to merge 2 commits into
patrykkopycinski wants to merge 2 commits into
Conversation
Contributor
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
6 tasks
patrykkopycinski
force-pushed
the
fix/cases-unified-attachment-gaps
branch
from
9d69b25 to
0546284
Compare
Contributor
Author
|
/ci |
…bled` is true When the unified attachments feature flag (`xpack.cases.attachments.enabled`) is enabled, several code paths in the Cases plugin still hardcode the legacy `cases-comments` Saved Object type, causing unified attachments stored in `cases-attachments` to be invisible, unreadable, or undeletable. This PR fixes all identified gaps to ensure end-to-end support for unified value attachments (find, get, bulkGet, delete, resolve, encode, and UI rendering).
patrykkopycinski
force-pushed
the
fix/cases-unified-attachment-gaps
branch
from
0546284 to
6caaa6f
Compare
Contributor
Author
|
/ci |
Contributor
💔 Build Failed
Failed CI Steps
Test Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
Contributor
|
Hey @patrykkopycinski, thanks for putting this together, just FYI, I just opened #256133 that addressed these gaps. it is an intended part 2 for the attachment V2 work |
Contributor
Author
|
Thank you @christineweng, looking forward for V2 attachments 💪 |
Contributor
|
Hey @patrykkopycinski! #256133 is merged so these gaps are filled. just FYI the feature flag for V2 is set to enabled for 9.5 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
xpack.cases.attachments.enabledistrue, unified value attachments (stored ascases-attachmentsSO type) are created correctly but cannot be displayed, fetched, or deleted through the existing Cases plugin code paths. This is because multiple server-side methods and client-side utilities still hardcodecases-commentsas the only SO type.This PR fixes all identified gaps to provide end-to-end support for unified value attachments.
Gaps Found & Fixes Applied
1.
AttachmentService.find()— only queriescases-commentsFile:
server/services/attachments/index.tsGap: The
find()method hardcodestype: CASE_COMMENT_SAVED_OBJECTwhen querying the SO client, so unified attachments stored ascases-attachmentsare invisible.Fix: Dynamically determine types based on
getAttachmentSavedObjectType(config). When unified is enabled, queries both[CASE_ATTACHMENT_SAVED_OBJECT, CASE_COMMENT_SAVED_OBJECT]. Decodes each result using the appropriate schema (UnifiedAttachmentAttributesRtfor unified,AttachmentAttributesRtV2for legacy).2.
AttachmentGetter.get()— only queriescases-commentsFile:
server/services/attachments/operations/get.tsGap: The single-attachment
get()method hardcodesCASE_COMMENT_SAVED_OBJECT, causing 404s when trying to get a unified attachment (e.g., during delete).Fix: When unified is enabled, try
CASE_ATTACHMENT_SAVED_OBJECTfirst; fall back toCASE_COMMENT_SAVED_OBJECT. Decode with the appropriate schema based onso.type.3.
AttachmentGetter.bulkGet()— only queriescases-commentsFile:
server/services/attachments/operations/get.tsGap: The
bulkGet()method (used by user actions timeline to populatelatestAttachments) only queriesCASE_COMMENT_SAVED_OBJECT. User action references may point to attachments that exist incases-attachments.Fix: When unified is enabled, issue two parallel
bulkGetcalls (one for each SO type) and merge results, preferring the new type when found.transformAndDecodeBulkGetResponsenow handlesCASE_ATTACHMENT_SAVED_OBJECTtypes by decoding withUnifiedAttachmentAttributesRt.4.
client/attachments/bulk_get.ts— authorization assumesownerfieldFile:
server/client/attachments/bulk_get.tsGap: Authorization logic calls
getAndEnsureAuthorizedEntitieson all attachments, but unified attachments don't have anownerfield. Also,flattenCommentSavedObjectsdoesn't handle unified SO shape.Fix: Partition fetched attachments into
unifiedAttachmentsandlegacyAttachments. Only apply authorization to legacy. Flatten unified attachments separately (direct attribute spread). Combine both lists.5.
BulkGetAttachmentsResponseRt— v1-only decoderFile:
common/types/api/attachment/v1.tsGap: The
BulkGetAttachmentsResponseRt.attachmentsfield usesAttachmentsRt(v1-only), which fails to decode unified attachments.Fix: Changed to
rt.array(AttachmentRtV2)which accepts both v1 and unified attachment shapes.6.
UserActionInternalFindResponse.latestAttachments— v1-only typeFile:
common/types/api/user_action/v1.tsGap: The
latestAttachmentsinterface field was typed asAttachments(v1-only), causing type mismatch with the now-widened bulk get response.Fix: Changed to
BulkGetAttachmentsResponse['attachments'].7.
convertAttachmentsToCamelCase— v1-only input typeFile:
public/api/utils.tsGap: Parameter accepts only
Attachment[], but the server now returns a mix of v1 and unified attachments.Fix: Widened to
Array<Attachment | AttachmentRequestV2>.8. Case resolve — v1 decoder fails on unified attachments
File:
server/client/cases/get.tsGap: The
resolve()function passes all comments (including unified) toCaseResolveResponseRt, which is a v1-only decoder. Unified attachments lackcomment,owner, etc. fields, causing decode errors.Fix: Filter out unified attachments from the
commentsarray before passing to the v1 decoder. Unified attachments are rendered separately via theunifiedAttachmentTypeRegistryin the UI.9. Case encode — v1 decoder fails on unified attachments
File:
server/common/models/case_with_comments.tsGap: Same issue as resolve —
encodeWithComments()passes all SOs toflattenCommentSavedObjects→CaseRt, which chokes on unified shapes.Fix: Filter to
CASE_COMMENT_SAVED_OBJECTonly before flattening.10.
getAll— authorization assumesownerfieldFile:
server/client/attachments/get.tsGap:
ensureSavedObjectsAreAuthorizedmaps over all SOs assuming.attributes.ownerexists. Also,flattenCommentSavedObjects→AttachmentsRtfails on unified shapes.Fix: Filter to legacy comments only for authorization and flattening.
11. Delete single attachment —
get()+owner+ decoder failuresFile:
server/client/attachments/delete.tsGap:
deleteComment()callsget()(gap #2), then accesses.attributes.owner(undefined for unified), runsAttachmentRequestRtdecoder (fails for unified), and creates a user action withowner(undefined).Fix: Check
attachment.type === CASE_ATTACHMENT_SAVED_OBJECTto detect unified. Skip authorization, v1 decode, user action creation, and alert handling for unified attachments (they have no owner, no alert fields).12. Delete all attachments —
owneraccess on unified SOsFile:
server/client/attachments/delete.tsGap:
deleteAll()maps over all SOs to getcomment.attributes.ownerfor authorization and user action creation. Crashes on unified attachments.Fix: Partition into legacy vs unified. Only run authorization/user-action/alert logic on legacy.
bulkDeletealready handles both types.13.
CaseCommentModel— user action methods assume legacy shapeFile:
server/common/models/case_with_comments.tsGap:
createCommentUserActionassumesAttachmentRequesttype (legacy). Unified attachments useUnifiedAttachmentPayloadand need a separate user action path.Fix: Split into
createLegacyCommentUserActionandcreateUnifiedCommentUserAction. Route based onisLegacyAttachmentRequest()check. Also passownerthrough tocreate/bulkCreateservice calls.14. UI — unified attachment rendering in user actions timeline
Files:
public/components/user_actions/comment/comment.tsx,unified_value.tsx,registered_attachments.tsx,types.ts,mock.ts,user_actions_list.tsxGap: The comment builder switch statement doesn't handle unified attachment types. The
UserActionBuilderArgsdoesn't includeunifiedAttachmentTypeRegistry.Fix: Added
unifiedAttachmentTypeRegistryto builder args. In thedefaultcase, check if the attachment type is registered in the unified registry and delegate tocreateUnifiedValueAttachmentUserActionBuilder. Createdunified_value.tsxto bridge unified attachments into the existingcreateRegisteredAttachmentUserActionBuilder. Widened the generic constraint oncreateRegisteredAttachmentUserActionBuilderto acceptAttachment | UnifiedAttachment.Test plan
xpack.cases.attachments.enabled: trueinkibana.dev.ymlyarn test:jest x-pack/platform/plugins/shared/cases