[Response Ops][Cases] Cases with empty string assignees throwing error by jcger · Pull Request #209973 · elastic/kibana

jcger · 2025-02-06T10:11:29Z

Summary

Closes #209950

Testing steps in referenced issue

Release note

Fix error message in cases list if case assignee is an empty string

elasticmachine · 2025-02-06T10:11:32Z

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

elasticmachine · 2025-02-06T10:11:32Z

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

cnasikas

Should we also remove empty strings when someone creates or update a case with assignees on the backend?

cnasikas · 2025-02-10T10:47:35Z

x-pack/platform/plugins/shared/cases/public/components/all_cases/all_cases_list.tsx

        for (const assignee of caseInfo.assignees) {
-          acc.add(assignee.uid);
+          if (!isEmpty(assignee.uid)) {
+            acc.add(assignee.uid);


What do you think of moving the responsibility to the useBulkGetUserProfiles hook?

We'll add it to bulkGetUserProfiles in x-pack/platform/plugins/shared/cases/public/containers/user_profiles/api.ts

jcger · 2025-02-10T13:31:55Z

Should we also remove empty strings when someone creates or update a case with assignees on the backend?

I think that would make sense, yes. Should we shut down this option completely by updating the schema definition or should we allow it but ignore? I think it was never our intention to have empty strings as profile uid but now that it seems like some users might have it, should we just filter out empty values like we do in the UI? @cnasikas

I tried

export const CaseUserProfileRt = rt.strict({
  uid: rt.refinement(rt.string, (s) => !isEmpty(s), 'nonEmptyString'),
});

I've seen that the cases list does not render any case when a case has an assignee with empty string because the _search api throws an error.

cnasikas · 2025-02-10T14:37:55Z

I think that would make sense, yes. Should we shut down this option completely by updating the schema definition or should we allow it but ignore?

I would say to ignore it and filter out empty values from the assignees array on the Create and Update Case API. Otherwise, it would be a breaking change.

…jcger/kibana into issue-209950-cases-list-broken-assignee

jcger · 2025-02-11T10:42:17Z

x-pack/platform/plugins/shared/cases/server/services/cases/sanitizers.ts

@@ -0,0 +1,36 @@
+/*


I created this sanitizers file for functions that clean request params. I saw there is also transformations but I think it's not the same. I also though it's important to add versioning to this functions as if there is a breaking change in the param, there would also be one here. Let me know if this should be done somewhere else.

I like the idea of sanitizers. Cases are structured in three layers like routes -> cases client -> services. In my previous comment, I suggested moving the responsibility of sanitizing the assignee's array in the cases client. In this case, it is better to have the sanitizers file in x-pack/platform/plugins/shared/cases/server/client/sanitizers.ts as we do for the validators.ts. Lastly, in the cases client, we do not version functions, so it is safe to have the sanitizers without versioning.

I'm thinking about moving this to .../cases/server/client/cases/ instead. I won't have to update anything, in .../cases/server/client/ it should be something more generic as there are assignees, at least, in user-actions. Seems like we follow the same pattern for validators too.

Sounds good to me!

jcger · 2025-02-11T10:43:06Z

x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/patch_case.ts

@@ -0,0 +1,54 @@
+/*


test in trial because only premium users can assign cases to a user

jcger · 2025-02-11T10:44:52Z

x-pack/platform/plugins/shared/cases/server/routes/api/cases/post_case.ts

+      const theCase = emptyCaseAssigneesSanitizerV1(rawCase);

-      const res: caseDomainV1.Case = await casesClient.cases.create({ ...theCase });
+      const res: caseDomainV1.Case = await casesClient.cases.create(theCase);


not sure why theCase was spread before but as theCase is now a new object anyways I though it would be ok to pass it through this way

It also seems ok to me.

jcger · 2025-02-11T15:41:50Z

x-pack/platform/plugins/shared/cases/public/components/all_cases/all_cases_list.test.tsx

  } as unknown as ReturnType<typeof useKibana>);
 };

+// eslint-disable-next-line prefer-object-spread


changed this initially, not needed anymore but keeping just because it's cleaner/easier to reuse if needed

cnasikas

LGTM! Do not forget about this comment: https://github.com/elastic/kibana/pull/209973/files#r1950814056.

elasticmachine · 2025-02-11T17:22:58Z

💚 Build Succeeded

Buildkite Build
Commit: 1bd6564

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`	540.8KB	540.9KB	+124.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`@kbn/test-suites-xpack`	719	721	+2

Total ESLint disabled count

id	before	after	diff
`@kbn/test-suites-xpack`	744	746	+2

History

💛 Build #275053 was flaky 68426ee
💔 Build #273922 failed 24146dc

kibanamachine · 2025-02-12T06:06:59Z

Starting backport for target branches: 8.16, 8.17, 8.18, 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/13279157956

elastic#209973) ## Summary Closes elastic#209950 Testing steps in referenced issue ## Release note Fix error message in cases list if case assignee is an empty string (cherry picked from commit 0897d08)

kibanamachine · 2025-02-12T06:13:24Z

💔 Some backports could not be created

Status	Branch	Result
❌	8.16	Backport failed because of merge conflicts
❌	8.17	Backport failed because of merge conflicts
✅	8.18
✅	8.x
✅	9.0

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 209973

Questions ?

Please refer to the Backport tool documentation

…g error (#209973) (#210733) # Backport This will backport the following commits from `main` to `9.0`: - [[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)](#209973)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>

…g error (#209973) (#210732) # Backport This will backport the following commits from `main` to `8.x`: - [[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)](#209973)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>

jcger · 2025-02-12T09:13:26Z

💔 Some backports could not be created

Status	Branch	Result
✅	8.17
❌	8.16	An unhandled error occurred. Please see the logs for details

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 209973

Questions ?

Please refer to the Backport tool documentation

elastic#209973) ## Summary Closes elastic#209950 Testing steps in referenced issue ## Release note Fix error message in cases list if case assignee is an empty string (cherry picked from commit 0897d08) # Conflicts: # x-pack/plugins/cases/server/client/cases/bulk_update.test.ts # x-pack/plugins/cases/server/client/cases/create.test.ts # x-pack/plugins/cases/server/client/cases/sanitizers.ts

jcger · 2025-02-12T10:06:34Z

💚 All backports created successfully

Status	Branch	Result
✅	8.16

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

Questions ?

Please refer to the Backport tool documentation

elastic#209973) ## Summary Closes elastic#209950 Testing steps in referenced issue ## Release note Fix error message in cases list if case assignee is an empty string (cherry picked from commit 0897d08) # Conflicts: # x-pack/plugins/cases/public/components/all_cases/all_cases_list.test.tsx # x-pack/plugins/cases/server/client/cases/bulk_update.test.ts # x-pack/plugins/cases/server/client/cases/create.test.ts # x-pack/plugins/cases/server/client/cases/sanitizers.ts

…ng error (#209973) (#210755) # Backport This will backport the following commits from `main` to `8.17`: - [[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)](#209973)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) \n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>"}},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210731","number":210731,"state":"OPEN"},{"branch":"8.16","label":"v8.16.4","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209973","number":209973,"mergeCommit":{"message":"[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)\n\n## Summary\nCloses https://github.com/elastic/kibana/issues/209950\n\nTesting steps in referenced issue\n\n## Release note\nFix error message in cases list if case assignee is an empty string","sha":"0897d0878622db9d9747e2bc1292d647c09ad996"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210732","number":210732,"state":"MERGED","mergeCommit":{"sha":"8b98614e9dbdaaeaebac56d633cd2b7c35af01bd","message":"[8.x] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) (#210732)\n\n# Backport\n\nThis will backport the following commits from `main` to `8.x`:\n- [[Response Ops][Cases] Cases with empty string assignees throwing\nerror (#209973)](https://github.com/elastic/kibana/pull/209973)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>"}}]}] BACKPORT-->

…ng error (#209973) (#210731) # Backport This will backport the following commits from `main` to `8.18`: - [[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)](#209973)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>

…ng error (#209973) (#210768) # Backport This will backport the following commits from `main` to `8.16`: - [[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)](#209973)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) \n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>"}},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210731","number":210731,"state":"OPEN"},{"branch":"8.16","label":"v8.16.4","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210755","number":210755,"state":"OPEN"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209973","number":209973,"mergeCommit":{"message":"[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)\n\n## Summary\nCloses https://github.com/elastic/kibana/issues/209950\n\nTesting steps in referenced issue\n\n## Release note\nFix error message in cases list if case assignee is an empty string","sha":"0897d0878622db9d9747e2bc1292d647c09ad996"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/210732","number":210732,"state":"MERGED","mergeCommit":{"sha":"8b98614e9dbdaaeaebac56d633cd2b7c35af01bd","message":"[8.x] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) (#210732)\n\n# Backport\n\nThis will backport the following commits from `main` to `8.x`:\n- [[Response Ops][Cases] Cases with empty string assignees throwing\nerror (#209973)](https://github.com/elastic/kibana/pull/209973)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>"}}]}] BACKPORT-->

filter empty assignees in cases list

24146dc

jcger added release_note:fix Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// Feature:Cases Cases feature backport:prev-major v9.1.0 labels Feb 6, 2025

jcger requested a review from a team as a code owner February 6, 2025 10:11

jcger added 2 commits February 7, 2025 07:38

Merge branch 'main' into issue-209950-cases-list-broken-assignee

9498aab

Merge branch 'main' into issue-209950-cases-list-broken-assignee

e4c340e

cnasikas self-requested a review February 10, 2025 08:10

cnasikas reviewed Feb 10, 2025

View reviewed changes

cnasikas added bug Fixes for quality problems that affect the customer experience v9.0.0 v8.18.0 v8.16.4 v8.17.2 labels Feb 10, 2025

jcger added 3 commits February 10, 2025 21:05

filter out empty uids for post and patch case

743d8f1

Merge branch 'issue-209950-cases-list-broken-assignee' of github.com:…

a68716a

…jcger/kibana into issue-209950-cases-list-broken-assignee

add tests

c59a06e

jcger commented Feb 11, 2025

View reviewed changes

jcger added 2 commits February 11, 2025 11:50

add api test

fc56a08

Merge branch 'main' into issue-209950-cases-list-broken-assignee

68426ee

jcger requested a review from cnasikas February 11, 2025 11:28

move sanitizer to client level

1bd6564

jcger commented Feb 11, 2025

View reviewed changes

cnasikas approved these changes Feb 11, 2025

View reviewed changes

jcger merged commit 0897d08 into elastic:main Feb 12, 2025

kibanamachine mentioned this pull request Feb 12, 2025

[8.18] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) #210731

Merged

kibanamachine mentioned this pull request Feb 12, 2025

[8.x] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) #210732

Merged

kibanamachine mentioned this pull request Feb 12, 2025

[9.0] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) #210733

Merged

kibanamachine added the v8.19.0 label Feb 12, 2025

jcger mentioned this pull request Feb 12, 2025

[8.17] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) #210755

Merged

jcger mentioned this pull request Feb 12, 2025

[8.16] [Response Ops][Cases] Cases with empty string assignees throwing error (#209973) #210768

Merged

kibanamachine added the v8.17.3 label Feb 12, 2025

kibanamachine mentioned this pull request Feb 12, 2025

[Lens][Embeddable] Make sure to not serialize searchSessionId #210765

Merged

1 task

kibanamachine added the v8.16.5 label Feb 12, 2025

cnasikas removed v8.16.4 v8.17.2 labels Feb 12, 2025

Conversation

jcger commented Feb 6, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Release note

Uh oh!

elasticmachine commented Feb 6, 2025

Uh oh!

elasticmachine commented Feb 6, 2025

Uh oh!

cnasikas left a comment

Choose a reason for hiding this comment

Uh oh!

cnasikas Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcger commented Feb 10, 2025

Uh oh!

cnasikas commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cnasikas left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 11, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

kibanamachine commented Feb 12, 2025

Uh oh!

kibanamachine commented Feb 12, 2025

💔 Some backports could not be created

Manual backport

Questions ?

Uh oh!

jcger commented Feb 12, 2025

💔 Some backports could not be created

Manual backport

Questions ?

Uh oh!

jcger commented Feb 12, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jcger commented Feb 6, 2025 •

edited by kibanamachine

Loading

cnasikas Feb 10, 2025 •

edited

Loading

cnasikas commented Feb 10, 2025 •

edited

Loading