diff --git a/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts b/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts index 3a194645c3fca..f55c1c78b1108 100644 --- a/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts +++ b/x-pack/plugins/cases/server/common/models/case_with_comments.test.ts @@ -6,11 +6,15 @@ */ import type { AlertAttachmentAttributes } from '../../../common/types/domain'; +import { AttachmentType } from '../../../common/types/domain'; import type { SavedObject } from '@kbn/core-saved-objects-api-server'; import { createCasesClientMockArgs } from '../../client/mocks'; import { alertComment, comment, mockCaseComments, mockCases, multipleAlert } from '../../mocks'; import { CaseCommentModel } from './case_with_comments'; -import { MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES } from '../../../common/constants'; +import { + MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES, + SECURITY_SOLUTION_OWNER, +} from '../../../common/constants'; import { commentExternalReference, commentFileExternalReference, @@ -25,6 +29,7 @@ describe('CaseCommentModel', () => { clientArgs.services.caseService.getCase.mockResolvedValue(theCase); clientArgs.services.caseService.patchCase.mockResolvedValue(theCase); clientArgs.services.attachmentService.create.mockResolvedValue(mockCaseComments[0]); + clientArgs.services.attachmentService.update.mockResolvedValue(mockCaseComments[0]); clientArgs.services.attachmentService.bulkCreate.mockResolvedValue({ saved_objects: mockCaseComments, }); @@ -274,6 +279,18 @@ describe('CaseCommentModel', () => { expect(clientArgs.services.attachmentService.create).not.toHaveBeenCalled(); }); + it('partial updates the case', async () => { + await model.createComment({ + id: 'comment-1', + commentReq: comment, + createdDate, + }); + + const args = clientArgs.services.caseService.patchCase.mock.calls[0][0]; + + expect(args.version).toBeUndefined(); + }); + describe('validation', () => { clientArgs.services.attachmentService.countPersistableStateAndExternalReferenceAttachments.mockResolvedValue( MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES @@ -579,6 +596,21 @@ describe('CaseCommentModel', () => { expect(multipleAlertsCall.attributes.index).toEqual(['test-index-3', 'test-index-5']); }); + it('partial updates the case', async () => { + await model.bulkCreate({ + attachments: [ + { + id: 'comment-1', + ...comment, + }, + ], + }); + + const args = clientArgs.services.caseService.patchCase.mock.calls[0][0]; + + expect(args.version).toBeUndefined(); + }); + describe('validation', () => { clientArgs.services.attachmentService.countPersistableStateAndExternalReferenceAttachments.mockResolvedValue( MAX_PERSISTABLE_STATE_AND_EXTERNAL_REFERENCES @@ -619,4 +651,24 @@ describe('CaseCommentModel', () => { }); }); }); + + describe('updateComment', () => { + it('partial updates the case', async () => { + await model.updateComment({ + updateRequest: { + id: 'comment-id', + version: 'comment-version', + type: AttachmentType.user, + comment: 'my updated comment', + owner: SECURITY_SOLUTION_OWNER, + }, + updatedAt: createdDate, + owner: SECURITY_SOLUTION_OWNER, + }); + + const args = clientArgs.services.caseService.patchCase.mock.calls[0][0]; + + expect(args.version).toBeUndefined(); + }); + }); }); diff --git a/x-pack/plugins/cases/server/common/models/case_with_comments.ts b/x-pack/plugins/cases/server/common/models/case_with_comments.ts index e1b89d7af791e..a1eb5cbfdb8b5 100644 --- a/x-pack/plugins/cases/server/common/models/case_with_comments.ts +++ b/x-pack/plugins/cases/server/common/models/case_with_comments.ts @@ -129,7 +129,7 @@ export class CaseCommentModel { }, options, }), - this.updateCaseUserAndDateSkipRefresh(updatedAt), + this.partialUpdateCaseUserAndDateSkipRefresh(updatedAt), ]); await commentableCase.createUpdateCommentUserAction(comment, updateRequest, owner); @@ -144,11 +144,11 @@ export class CaseCommentModel { } } - private async updateCaseUserAndDateSkipRefresh(date: string) { - return this.updateCaseUserAndDate(date, false); + private async partialUpdateCaseUserAndDateSkipRefresh(date: string) { + return this.partialUpdateCaseUserAndDate(date, false); } - private async updateCaseUserAndDate( + private async partialUpdateCaseUserAndDate( date: string, refresh: RefreshSetting ): Promise { @@ -160,7 +160,6 @@ export class CaseCommentModel { updated_at: date, updated_by: { ...this.params.user }, }, - version: this.caseInfo.version, refresh, }); @@ -242,7 +241,7 @@ export class CaseCommentModel { id, refresh: false, }), - this.updateCaseUserAndDateSkipRefresh(createdDate), + this.partialUpdateCaseUserAndDateSkipRefresh(createdDate), ]); await Promise.all([ @@ -502,7 +501,7 @@ export class CaseCommentModel { }), refresh: false, }), - this.updateCaseUserAndDateSkipRefresh(new Date().toISOString()), + this.partialUpdateCaseUserAndDateSkipRefresh(new Date().toISOString()), ]); const savedObjectsWithoutErrors = newlyCreatedAttachments.saved_objects.filter( diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts index 8c2c356b6090e..1ea5013b16506 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/patch_comment.ts @@ -12,6 +12,7 @@ import { AlertAttachmentAttributes, UserCommentAttachmentAttributes, AttachmentType, + CaseStatuses, } from '@kbn/cases-plugin/common/types/domain'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; @@ -34,6 +35,7 @@ import { updateComment, superUserSpace1Auth, removeServerGeneratedPropertiesFromSavedObject, + updateCase, } from '../../../../common/lib/api'; import { globalRead, @@ -549,6 +551,48 @@ export default ({ getService }: FtrProviderContext): void => { } }); + describe('partial updates', () => { + it('should not result to a version conflict (409) when updating a comment to an updated case', async () => { + const postedCase = await createCase(supertest, postCaseReq); + const caseWithComments = await createComment({ + supertest, + caseId: postedCase.id, + params: postCommentUserReq, + expectedHttpCode: 200, + }); + + /** + * Updating the status of the case will + * change the version of the case + */ + await updateCase({ + supertest, + params: { + cases: [ + { + id: caseWithComments.id, + version: caseWithComments.version, + status: CaseStatuses['in-progress'], + }, + ], + }, + }); + + await updateComment({ + supertest, + caseId: postedCase.id, + req: { + id: caseWithComments.comments![0].id, + version: caseWithComments.comments![0].version, + comment: 'my new comment', + type: AttachmentType.user, + owner: 'securitySolutionFixture', + }, + expectedHttpCode: 200, + }); + }); + }); + describe('rbac', () => { const supertestWithoutAuth = getService('supertestWithoutAuth'); diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts index 88723df678f8a..fcb376e4df522 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/comments/post_comment.ts @@ -1123,6 +1123,36 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + describe('partial updates', () => { + it('should not result to a version conflict (409) when adding a comment to an updated case', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + /** + * Updating the status of the case will + * change the version of the case + */ + await updateCase({ + supertest, + params: { + cases: [ + { + id: postedCase.id, + version: postedCase.version, + status: CaseStatuses['in-progress'], + }, + ], + }, + }); + + await createComment({ + supertest, + caseId: postedCase.id, + params: postCommentUserReq, + expectedHttpCode: 200, + }); + }); + }); + describe('rbac', () => { afterEach(async () => { await deleteAllCaseItems(es); diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts index 3c3748a844ea6..6c929f67b8c90 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts @@ -1512,6 +1512,36 @@ export default ({ getService }: FtrProviderContext): void => { }); }); + describe('partial updates', () => { + it('should not result to a version conflict (409) when adding comments to an updated case', async () => { + const postedCase = await createCase(supertest, postCaseReq); + + /** + * Updating the status of the case will + * change the version of the case + */ + await updateCase({ + supertest, + params: { + cases: [ + { + id: postedCase.id, + version: postedCase.version, + status: CaseStatuses['in-progress'], + }, + ], + }, + }); + + await bulkCreateAttachments({ + supertest, + caseId: postedCase.id, + params: [postCommentUserReq], + expectedHttpCode: 200, + }); + }); + }); + describe('rbac', () => { afterEach(async () => { await deleteAllCaseItems(es);