Skip to content

Commit 3a23a20

Browse files
cnasikaskibanamachine
cnasikas
and
kibanamachine
authored
[8.19] [ResponseOps][Cases] Populate total alerts and comments in the cases saved objects (elastic#223992) (elastic#224928)
# Backport This will backport the following commits from `main` to `8.19`: - [[ResponseOps][Cases] Populate total alerts and comments in the cases saved objects (elastic#223992)](elastic#223992) <!--- Backport version: 10.0.1 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Christos Nasikas","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-06-23T16:56:01Z","message":"[ResponseOps][Cases] Populate total alerts and comments in the cases saved objects (elastic#223992)\n\n## Summary\n\nThis is a farewell PR to Cases. Probably my last PR to the cases\ncodebase. It was quite a journey, and I learned a lot. I hope the best\nfor the feature of Cases.\n\n## Decisions\n\nJust before Cases was forbidden to do migrations, we did a last\nmigration to all cases to persist `total_alerts: -1` and\n`total_comments: -1`. We did that so that in the future, when we would\nwant to populate the fields, we would know which cases have their fields\npopulated and which do not. In this PR, due to time constraints and\ncriticality of the feature, I took the following decisions:\n\n- Cases return from their APIs the total comments and alerts of each\ncase. They do that by doing an aggregation, getting the counts, and\nmerging them with the response. I did not change that behavior. In\nfollowing PRs, it can be optimized and fetch the stats only for cases\nthat do not yet have their stats populated (cases with -1 in the counts)\n- When a case is created, the counts are zero.\n- When a comment or alert is added, I do an aggregation to get the stats\n(total alerts and comments) of the current case, and then update the\ncounters with the number of the newly created attachments. The case is\nupdated without version checks. In race conditions, where an attachment\nis being added before updating the case, the numbers could be off. This\nis a deliberate choice. It can be fixed later with retries and version\nconcurrency control.\n- The case service will continue to not return the `total_alerts` and\n`total_comments`.\n- The case service will accept the `total_alerts` and `total_comments`\nattributes to be able to set them.\n\nFixes: https://github.com/elastic/kibana/issues/217636\n\ncc @michaelolo24 \n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"f30335ac3d74bd3310167a27d035544c72068111","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","Feature:Cases","backport:version","v9.1.0","v8.19.0"],"title":"[ResponseOps][Cases] Populate total alerts and comments in the cases saved objects","number":223992,"url":"https://github.com/elastic/kibana/pull/223992","mergeCommit":{"message":"[ResponseOps][Cases] Populate total alerts and comments in the cases saved objects (elastic#223992)\n\n## Summary\n\nThis is a farewell PR to Cases. Probably my last PR to the cases\ncodebase. It was quite a journey, and I learned a lot. I hope the best\nfor the feature of Cases.\n\n## Decisions\n\nJust before Cases was forbidden to do migrations, we did a last\nmigration to all cases to persist `total_alerts: -1` and\n`total_comments: -1`. We did that so that in the future, when we would\nwant to populate the fields, we would know which cases have their fields\npopulated and which do not. In this PR, due to time constraints and\ncriticality of the feature, I took the following decisions:\n\n- Cases return from their APIs the total comments and alerts of each\ncase. They do that by doing an aggregation, getting the counts, and\nmerging them with the response. I did not change that behavior. In\nfollowing PRs, it can be optimized and fetch the stats only for cases\nthat do not yet have their stats populated (cases with -1 in the counts)\n- When a case is created, the counts are zero.\n- When a comment or alert is added, I do an aggregation to get the stats\n(total alerts and comments) of the current case, and then update the\ncounters with the number of the newly created attachments. The case is\nupdated without version checks. In race conditions, where an attachment\nis being added before updating the case, the numbers could be off. This\nis a deliberate choice. It can be fixed later with retries and version\nconcurrency control.\n- The case service will continue to not return the `total_alerts` and\n`total_comments`.\n- The case service will accept the `total_alerts` and `total_comments`\nattributes to be able to set them.\n\nFixes: https://github.com/elastic/kibana/issues/217636\n\ncc @michaelolo24 \n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"f30335ac3d74bd3310167a27d035544c72068111"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/223992","number":223992,"mergeCommit":{"message":"[ResponseOps][Cases] Populate total alerts and comments in the cases saved objects (elastic#223992)\n\n## Summary\n\nThis is a farewell PR to Cases. Probably my last PR to the cases\ncodebase. It was quite a journey, and I learned a lot. I hope the best\nfor the feature of Cases.\n\n## Decisions\n\nJust before Cases was forbidden to do migrations, we did a last\nmigration to all cases to persist `total_alerts: -1` and\n`total_comments: -1`. We did that so that in the future, when we would\nwant to populate the fields, we would know which cases have their fields\npopulated and which do not. In this PR, due to time constraints and\ncriticality of the feature, I took the following decisions:\n\n- Cases return from their APIs the total comments and alerts of each\ncase. They do that by doing an aggregation, getting the counts, and\nmerging them with the response. I did not change that behavior. In\nfollowing PRs, it can be optimized and fetch the stats only for cases\nthat do not yet have their stats populated (cases with -1 in the counts)\n- When a case is created, the counts are zero.\n- When a comment or alert is added, I do an aggregation to get the stats\n(total alerts and comments) of the current case, and then update the\ncounters with the number of the newly created attachments. The case is\nupdated without version checks. In race conditions, where an attachment\nis being added before updating the case, the numbers could be off. This\nis a deliberate choice. It can be fixed later with retries and version\nconcurrency control.\n- The case service will continue to not return the `total_alerts` and\n`total_comments`.\n- The case service will accept the `total_alerts` and `total_comments`\nattributes to be able to set them.\n\nFixes: https://github.com/elastic/kibana/issues/217636\n\ncc @michaelolo24 \n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"f30335ac3d74bd3310167a27d035544c72068111"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <[email protected]>
1 parent 35f797a commit 3a23a20

File tree

24 files changed

+834
-89
lines changed

24 files changed

+834
-89
lines changed

‎x-pack/platform/plugins/shared/cases/server/client/attachments/delete.test.ts‎

Lines changed: 105 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,30 @@ describe('delete', () => {
1515

1616
beforeEach(() => {
1717
jest.clearAllMocks();
18+
jest.resetAllMocks();
19+
20+
clientArgs.services.attachmentService.getter.get.mockResolvedValue(mockCaseComments[0]);
21+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
22+
new Map()
23+
);
24+
});
25+
26+
it('refreshes when deleting', async () => {
27+
await deleteComment({ caseID: 'mock-id-1', attachmentID: 'mock-comment-1' }, clientArgs);
28+
29+
expect(clientArgs.services.attachmentService.bulkDelete).toHaveBeenCalledWith({
30+
attachmentIds: ['mock-comment-1'],
31+
refresh: true,
32+
});
1833
});
1934

2035
describe('Alerts', () => {
2136
const commentSO = mockCaseComments[0];
2237
const alertsSO = mockCaseComments[3];
23-
clientArgs.services.attachmentService.getter.get.mockResolvedValue(alertsSO);
38+
39+
beforeEach(() => {
40+
clientArgs.services.attachmentService.getter.get.mockResolvedValue(alertsSO);
41+
});
2442

2543
it('delete alerts correctly', async () => {
2644
await deleteComment({ caseID: 'mock-id-4', attachmentID: 'mock-comment-4' }, clientArgs);
@@ -38,10 +56,41 @@ describe('delete', () => {
3856
expect(clientArgs.services.alertsService.removeCaseIdFromAlerts).not.toHaveBeenCalledWith();
3957
});
4058
});
59+
60+
describe('Attachment stats', () => {
61+
it('updates attachment stats correctly', async () => {
62+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
63+
new Map([
64+
[
65+
'mock-id-1',
66+
{
67+
userComments: 2,
68+
alerts: 2,
69+
},
70+
],
71+
])
72+
);
73+
74+
await deleteComment({ caseID: 'mock-id-1', attachmentID: 'mock-comment-1' }, clientArgs);
75+
76+
const args = clientArgs.services.caseService.patchCase.mock.calls[0][0];
77+
78+
expect(args.updatedAttributes.total_comments).toEqual(2);
79+
expect(args.updatedAttributes.total_alerts).toEqual(2);
80+
expect(args.updatedAttributes.updated_at).toBeDefined();
81+
expect(args.updatedAttributes.updated_by).toEqual({
82+
email: '[email protected]',
83+
full_name: 'Damaged Raccoon',
84+
profile_uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
85+
username: 'damaged_raccoon',
86+
});
87+
});
88+
});
4189
});
4290

4391
describe('deleteAll', () => {
4492
const clientArgs = createCasesClientMockArgs();
93+
4594
const getAllCaseCommentsResponse = {
4695
saved_objects: mockCaseComments.map((so) => ({ ...so, score: 0 })),
4796
total: mockCaseComments.length,
@@ -51,13 +100,36 @@ describe('delete', () => {
51100

52101
beforeEach(() => {
53102
jest.clearAllMocks();
54-
});
103+
jest.resetAllMocks();
104+
105+
clientArgs.services.attachmentService.getter.get.mockResolvedValue(mockCaseComments[0]);
106+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
107+
new Map()
108+
);
55109

56-
describe('Alerts', () => {
57110
clientArgs.services.caseService.getAllCaseComments.mockResolvedValue(
58111
getAllCaseCommentsResponse
59112
);
113+
});
114+
115+
it('refreshes when deleting', async () => {
116+
await deleteAll({ caseID: 'mock-id-1' }, clientArgs);
60117

118+
expect(clientArgs.services.attachmentService.bulkDelete).toHaveBeenCalledWith({
119+
attachmentIds: [
120+
'mock-comment-1',
121+
'mock-comment-2',
122+
'mock-comment-3',
123+
'mock-comment-4',
124+
'mock-comment-5',
125+
'mock-comment-6',
126+
'mock-comment-7',
127+
],
128+
refresh: true,
129+
});
130+
});
131+
132+
describe('Alerts', () => {
61133
it('delete alerts correctly', async () => {
62134
await deleteAll({ caseID: 'mock-id-4' }, clientArgs);
63135

@@ -82,5 +154,35 @@ describe('delete', () => {
82154
expect(clientArgs.services.alertsService.removeCaseIdFromAlerts).not.toHaveBeenCalledWith();
83155
});
84156
});
157+
158+
describe('Attachment stats', () => {
159+
it('updates attachment stats correctly', async () => {
160+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
161+
new Map([
162+
[
163+
'mock-id-1',
164+
{
165+
userComments: 0,
166+
alerts: 0,
167+
},
168+
],
169+
])
170+
);
171+
172+
await deleteAll({ caseID: 'mock-id-1' }, clientArgs);
173+
174+
const args = clientArgs.services.caseService.patchCase.mock.calls[0][0];
175+
176+
expect(args.updatedAttributes.total_comments).toEqual(0);
177+
expect(args.updatedAttributes.total_alerts).toEqual(0);
178+
expect(args.updatedAttributes.updated_at).toBeDefined();
179+
expect(args.updatedAttributes.updated_by).toEqual({
180+
email: '[email protected]',
181+
full_name: 'Damaged Raccoon',
182+
profile_uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0',
183+
username: 'damaged_raccoon',
184+
});
185+
});
186+
});
85187
});
86188
});

‎x-pack/platform/plugins/shared/cases/server/client/attachments/delete.ts‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,14 @@ export async function deleteAll(
5252

5353
await attachmentService.bulkDelete({
5454
attachmentIds: comments.saved_objects.map((so) => so.id),
55-
refresh: false,
55+
refresh: true,
56+
});
57+
58+
await updateCaseAttachmentStats({
59+
caseService: clientArgs.services.caseService,
60+
attachmentService: clientArgs.services.attachmentService,
61+
caseId: caseID,
62+
user,
5663
});
5764

5865
await userActionService.creator.bulkCreateAttachmentDeletion({
@@ -115,7 +122,14 @@ export async function deleteComment(
115122

116123
await attachmentService.bulkDelete({
117124
attachmentIds: [attachmentID],
118-
refresh: false,
125+
refresh: true,
126+
});
127+
128+
await updateCaseAttachmentStats({
129+
caseService: clientArgs.services.caseService,
130+
attachmentService: clientArgs.services.attachmentService,
131+
caseId: caseID,
132+
user,
119133
});
120134

121135
// we only want to store the fields related to the original request of the attachment, not fields like
@@ -163,3 +177,42 @@ const handleAlerts = async ({ alertsService, attachments, caseId }: HandleAlerts
163177
const alerts = getAlertInfoFromComments(alertAttachments);
164178
await alertsService.removeCaseIdFromAlerts({ alerts, caseId });
165179
};
180+
181+
interface UpdateCaseAttachmentStats {
182+
caseService: CasesClientArgs['services']['caseService'];
183+
attachmentService: CasesClientArgs['services']['attachmentService'];
184+
caseId: string;
185+
user: CasesClientArgs['user'];
186+
}
187+
188+
const updateCaseAttachmentStats = async ({
189+
caseService,
190+
attachmentService,
191+
caseId,
192+
user,
193+
}: UpdateCaseAttachmentStats) => {
194+
const originalCase = await caseService.getCase({
195+
id: caseId,
196+
});
197+
198+
const date = new Date().toISOString();
199+
200+
const attachmentStats = await attachmentService.getter.getCaseAttatchmentStats({
201+
caseIds: [caseId],
202+
});
203+
204+
const totalComments = attachmentStats.get(caseId)?.userComments ?? 0;
205+
const totalAlerts = attachmentStats.get(caseId)?.alerts ?? 0;
206+
207+
await caseService.patchCase({
208+
originalCase,
209+
caseId,
210+
updatedAttributes: {
211+
updated_at: date,
212+
updated_by: user,
213+
total_comments: totalComments,
214+
total_alerts: totalAlerts,
215+
},
216+
refresh: false,
217+
});
218+
};

‎x-pack/platform/plugins/shared/cases/server/client/cases/bulk_get.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ describe('bulkGet', () => {
2121
unauthorized: [],
2222
});
2323

24-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
24+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
25+
new Map()
26+
);
2527

2628
beforeEach(() => {
2729
jest.clearAllMocks();

‎x-pack/platform/plugins/shared/cases/server/client/cases/bulk_get.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const bulkGet = async (
4949
operation: Operations.bulkGetCases,
5050
});
5151

52-
const commentTotals = await attachmentService.getter.getCaseCommentStats({
52+
const commentTotals = await attachmentService.getter.getCaseAttatchmentStats({
5353
caseIds: authorizedCases.map((theCase) => theCase.id),
5454
});
5555

‎x-pack/platform/plugins/shared/cases/server/client/cases/bulk_update.test.ts‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ describe('update', () => {
5353
saved_objects: [{ ...mockCases[0], attributes: { assignees: cases.cases[0].assignees } }],
5454
});
5555

56-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
56+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
57+
new Map()
58+
);
5759
});
5860

5961
it('notifies an assignee', async () => {
@@ -437,7 +439,9 @@ describe('update', () => {
437439
per_page: 10,
438440
page: 1,
439441
});
440-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
442+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
443+
new Map()
444+
);
441445
});
442446

443447
it(`does not throw error when category is non empty string less than ${MAX_CATEGORY_LENGTH} characters`, async () => {
@@ -571,7 +575,9 @@ describe('update', () => {
571575
per_page: 10,
572576
page: 1,
573577
});
574-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
578+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
579+
new Map()
580+
);
575581
});
576582

577583
it(`does not throw error when title is non empty string less than ${MAX_TITLE_LENGTH} characters`, async () => {
@@ -706,7 +712,9 @@ describe('update', () => {
706712
per_page: 10,
707713
page: 1,
708714
});
709-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
715+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
716+
new Map()
717+
);
710718
});
711719

712720
it(`does not throw error when description is non empty string less than ${MAX_DESCRIPTION_LENGTH} characters`, async () => {
@@ -848,7 +856,7 @@ describe('update', () => {
848856
const caseCommentsStats = new Map();
849857
caseCommentsStats.set(mockCases[0].id, { userComments: 1, alerts: 2 });
850858
caseCommentsStats.set(mockCases[1].id, { userComments: 3, alerts: 4 });
851-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(
859+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
852860
caseCommentsStats
853861
);
854862
});
@@ -972,7 +980,9 @@ describe('update', () => {
972980
]
973981
`);
974982

975-
expect(clientArgs.services.attachmentService.getter.getCaseCommentStats).toHaveBeenCalledWith(
983+
expect(
984+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats
985+
).toHaveBeenCalledWith(
976986
expect.objectContaining({
977987
caseIds: [mockCases[0].id, mockCases[1].id],
978988
})
@@ -992,7 +1002,9 @@ describe('update', () => {
9921002
per_page: 10,
9931003
page: 1,
9941004
});
995-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
1005+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
1006+
new Map()
1007+
);
9961008
});
9971009

9981010
it('does not throw error when tags array is empty', async () => {
@@ -1197,7 +1209,9 @@ describe('update', () => {
11971209
customFields: defaultCustomFieldsConfiguration,
11981210
},
11991211
]);
1200-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
1212+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
1213+
new Map()
1214+
);
12011215
});
12021216

12031217
it('can update customFields', async () => {
@@ -1587,7 +1601,7 @@ describe('update', () => {
15871601

15881602
beforeEach(() => {
15891603
jest.clearAllMocks();
1590-
clientArgsMock.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(
1604+
clientArgsMock.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
15911605
new Map()
15921606
);
15931607
});
@@ -1807,7 +1821,7 @@ describe('update', () => {
18071821
per_page: 10,
18081822
page: 1,
18091823
});
1810-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(
1824+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
18111825
new Map()
18121826
);
18131827
});
@@ -1915,7 +1929,9 @@ describe('update', () => {
19151929
saved_objects: mockCases,
19161930
});
19171931

1918-
clientArgs.services.attachmentService.getter.getCaseCommentStats.mockResolvedValue(new Map());
1932+
clientArgs.services.attachmentService.getter.getCaseAttatchmentStats.mockResolvedValue(
1933+
new Map()
1934+
);
19191935
});
19201936

19211937
it('calculates metrics correctly', async () => {

‎x-pack/platform/plugins/shared/cases/server/client/cases/bulk_update.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ export const bulkUpdate = async (
516516
alertsService,
517517
});
518518

519-
const commentsMap = await attachmentService.getter.getCaseCommentStats({
519+
const commentsMap = await attachmentService.getter.getCaseAttatchmentStats({
520520
caseIds,
521521
});
522522

‎x-pack/platform/plugins/shared/cases/server/client/cases/get.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ export const getCasesByAlertID = async (
103103
return [];
104104
}
105105

106-
const commentStats = await attachmentService.getter.getCaseCommentStats({
106+
const commentStats = await attachmentService.getter.getCaseAttatchmentStats({
107107
caseIds,
108108
});
109109

0 commit comments

Comments
 (0)