[Cases] Bulk edit tags: Prevent error when updating cases with the same tags #145608

cnasikas · 2022-11-17T18:37:41Z

Summary

This PR fixes a bug where an error occurs when updating the case with the same tags. To fix it, we filter cases without any changes.

Fixes: #145217

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2022-11-17T18:37:44Z

Pinging @elastic/response-ops (Team:ResponseOps)

elasticmachine · 2022-11-17T18:37:45Z

Pinging @elastic/response-ops-cases (Feature:Cases)

cnasikas · 2022-11-17T18:38:31Z

x-pack/plugins/cases/public/components/actions/tags/use_tags_action.tsx

@@ -37,16 +38,26 @@ export const useTagsAction = ({ onAction, onActionSuccess, isDisabled }: UseActi
    (tagsSelection: TagsSelectionState) => {
      onAction();
      onFlyoutClosed();
-      const casesToUpdate = selectedCasesToEditTags.map((theCase) => {
+
+      const casesToUpdate = selectedCasesToEditTags.reduce((acc, theCase) => {


I changed it to reduce to map and filter at the same time

cnasikas · 2022-11-17T18:38:57Z

x-pack/plugins/cases/public/components/actions/tags/use_tags_action.tsx

        const tags = difference(theCase.tags, tagsSelection.unSelectedTags);
        const uniqueTags = new Set([...tags, ...tagsSelection.selectedTags]);
+        const uniqueTagsAsArray = Array.from(uniqueTags.values());
+
+        if (isEqual(theCase.tags, uniqueTagsAsArray)) {


No need to update the case if the original tags are the same as the new ones

How would sorting work for these arrays? If the arrays contain the same strings but are in a different order isEqual will return false. Maybe that's ok though, I'd have to check the backend to see if it would still throw an error in that situation. I suppose ideally we'd just skip the update since reordering the tags doesn't really do much in the UI.

Thanks, I was not aware of that. I fixed it here a200227. As we have already a set of uniqueTags I iterate through the theCase.tags and check if it is a member of the uniqueTags set. If all of the elements are part of the set then the tags are identical and there is no need to do an update.

x-pack/plugins/cases/public/containers/use_bulk_update_case.tsx

jonathan-buttner · 2022-11-17T20:35:54Z

x-pack/plugins/cases/public/components/actions/tags/use_tags_action.tsx

        const tags = difference(theCase.tags, tagsSelection.unSelectedTags);
        const uniqueTags = new Set([...tags, ...tagsSelection.selectedTags]);
+        const uniqueTagsAsArray = Array.from(uniqueTags.values());
+
+        if (isEqual(theCase.tags, uniqueTagsAsArray)) {


How would sorting work for these arrays? If the arrays contain the same strings but are in a different order isEqual will return false. Maybe that's ok though, I'd have to check the backend to see if it would still throw an error in that situation. I suppose ideally we'd just skip the update since reordering the tags doesn't really do much in the UI.

jonathan-buttner · 2022-11-17T20:39:43Z

x-pack/plugins/cases/public/components/actions/tags/use_tags_action.tsx

+        const uniqueTagsAsArray = Array.from(uniqueTags.values());
+
+        if (isEqual(theCase.tags, uniqueTagsAsArray)) {
+          onActionSuccess();


Why do we want to call the success callback here? Would that trigger a refresh of the cases?

https://github.com/elastic/kibana/blob/main/x-pack/plugins/cases/public/components/all_cases/use_actions.tsx#L61

const tagsAction = useTagsAction({ isDisabled: false, onAction: closePopover, onActionSuccess: refreshCases, });

My mistake! I forgot to remove it.

peteharverson · 2022-11-18T12:48:52Z

x-pack/plugins/cases/public/components/actions/tags/use_tags_action.tsx

+            version: theCase.version,
+          },
+        ];
+      }, [] as CaseUpdateRequest[]);


As discussed with @cnasikas , I still see the error toast as an empty array casesToUpdate is getting sent to the backend.

Fixed in a200227

Confirmed fixed for the bulk edit of tags.

Note this same type of issue still occurs with the View case page - would be good to fix this in a follow-up PR:

Created #145736 to fix this in the View case page.

peteharverson

Tested latest edits and LGTM

adcoelho · 2022-11-18T15:28:15Z

x-pack/plugins/cases/public/components/actions/tags/use_tags_action.tsx

-        };
-      });
+      const casesToUpdate = selectedCasesToEditTags.reduce((acc, theCase) => {
+        const tagsWithoutUnselectedTags = difference(theCase.tags, tagsSelection.unSelectedTags);


If you make this one a set too you can just use lodash's .isEqual() instead of the aux function.

kibana-ci · 2022-11-21T15:51:01Z

💚 Build Succeeded

Buildkite Build
Commit: f01f830

Metrics [docs]

Async chunks

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

id	before	after	diff
`cases`	347.3KB	347.4KB	+78.0B

Page load bundle

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

id	before	after	diff
`cases`	126.8KB	127.0KB	+162.0B

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	59	65	+6
`osquery`	109	115	+6
`securitySolution`	442	448	+6
total			+20

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	67	73	+6
`osquery`	110	117	+7
`securitySolution`	519	525	+6
total			+21

History

💔 Build #89222 failed a200227
💔 Build #88956 failed 8fb0c39

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @cnasikas

…me tags (elastic#145608) ## Summary This PR fixes a bug where an error occurs when updating the case with the same tags. To fix it, we filter cases without any changes. Fixes: elastic#145217 ### Checklist Delete any items that are not applicable to this PR. - [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 ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit b1fb286)

kibanamachine · 2022-11-21T16:00:13Z

💚 All backports created successfully

Status	Branch	Result
✅	8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…the same tags (#145608) (#145886) # Backport This will backport the following commits from `main` to `8.6`: - [[Cases] Bulk edit tags: Prevent error when updating cases with the same tags (#145608)](#145608)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co>

cnasikas added 2 commits November 17, 2022 20:28

Do not update when the tags are the same

e0f29b6

Fix toaster title

8fb0c39

cnasikas added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.6.0 v8.7.0 labels Nov 17, 2022

cnasikas requested a review from a team as a code owner November 17, 2022 18:37

cnasikas self-assigned this Nov 17, 2022

cnasikas commented Nov 17, 2022

View reviewed changes

x-pack/plugins/cases/public/containers/use_bulk_update_case.tsx Show resolved Hide resolved

cnasikas mentioned this pull request Nov 17, 2022

[Security Solution]Error prompt on editing tag under cases #145217

Closed

jonathan-buttner reviewed Nov 17, 2022

View reviewed changes

peteharverson reviewed Nov 18, 2022

View reviewed changes

Fix bug with empty cases

a200227

peteharverson mentioned this pull request Nov 18, 2022

[Cases] Error toast shown when saving unchanged tags in view case page in the bulk edit of tags. #145736

Open

peteharverson approved these changes Nov 18, 2022

View reviewed changes

adcoelho reviewed Nov 18, 2022

View reviewed changes

jonathan-buttner approved these changes Nov 21, 2022

View reviewed changes

cnasikas added 2 commits November 21, 2022 16:48

Merge branch 'main' into fix_tags_bug

7dea56d

PR feedback

f01f830

cnasikas enabled auto-merge (squash) November 21, 2022 14:54

cnasikas merged commit b1fb286 into elastic:main Nov 21, 2022

cnasikas deleted the fix_tags_bug branch November 21, 2022 15:51

kibanamachine mentioned this pull request Nov 21, 2022

[8.6] [Cases] Bulk edit tags: Prevent error when updating cases with the same tags (#145608) #145886

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Bulk edit tags: Prevent error when updating cases with the same tags #145608

[Cases] Bulk edit tags: Prevent error when updating cases with the same tags #145608

cnasikas commented Nov 17, 2022 •

edited by kibanamachine

Loading

elasticmachine commented Nov 17, 2022

elasticmachine commented Nov 17, 2022

cnasikas Nov 17, 2022 •

edited

Loading

cnasikas Nov 17, 2022

jonathan-buttner Nov 17, 2022

cnasikas Nov 18, 2022 •

edited

Loading

jonathan-buttner Nov 17, 2022

jonathan-buttner Nov 17, 2022

cnasikas Nov 18, 2022

peteharverson Nov 18, 2022

cnasikas Nov 18, 2022

peteharverson Nov 18, 2022

peteharverson Nov 18, 2022

peteharverson left a comment

adcoelho Nov 18, 2022 •

edited

Loading

cnasikas Nov 21, 2022

kibana-ci commented Nov 21, 2022

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Nov 21, 2022

[Cases] Bulk edit tags: Prevent error when updating cases with the same tags #145608

[Cases] Bulk edit tags: Prevent error when updating cases with the same tags #145608

Conversation

cnasikas commented Nov 17, 2022 • edited by kibanamachine Loading

Summary

Checklist

For maintainers

elasticmachine commented Nov 17, 2022

elasticmachine commented Nov 17, 2022

cnasikas Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnasikas Nov 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

adcoelho Nov 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Nov 21, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Nov 21, 2022

💚 All backports created successfully

Questions ?

cnasikas commented Nov 17, 2022 •

edited by kibanamachine

Loading

cnasikas Nov 17, 2022 •

edited

Loading

cnasikas Nov 18, 2022 •

edited

Loading

adcoelho Nov 18, 2022 •

edited

Loading