[Cases] Add guardrails for add and update comment API

[js-jankisalvi]

Connected to #146945

Summary

Description	Limit	Done?	Documented?
Total number of comment characters	30.000	✅	Yes

• Tests.
• Updated Documentation.

Checklist

Delete any items that are not applicable to this PR.

• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios

Release Notes

The total number of characters per comment is limited to 30000

[elasticmachine]

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

[elasticmachine]

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

[lcawl]

OAS LGTM, thanks!

[cnasikas]

Nice work! Can you add a test in x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_create_attachments.ts?

[cnasikas]

Let's add some tests for the CommentRequestRt to verify the validation. In general, I think it is nice if we created a strong linked chain of tests. For example, we test the CommentRequestRt with all possible testing scenarios. Then we test the addComment and bulkCreate but there there is no need to check all possible testing scenarios (unless there is new logic on top of it). One is enough to ensure the integration between the function and the schema. The chain now is addComment -> CommentRequestRt. If the tests in CommentRequestRt pass we are sure that addComment will work correctly because we have at least one test that tests the integration between the two. Lastly, with integration tests, we check the integration between the addComment and the route.

This article explains the concept in detail: https://www.jamesshore.com/v2/projects/nullables/testing-without-mocks#sociable-tests

cc @adcoelho

[adcoelho]

Then we test the addComment and bulkCreate but there there is no need to check all possible testing scenarios (unless there is new logic on top of it).

I wondered about this but we need to guarantee somehow that (in this case) CommentRequestRt is being used by addComment and bulkCreate.

Is this what you mean with

because we have at least one test that tests the integration between the two.

?

[js-jankisalvi]

Ahh okay, so just checking if I understood it correctly,

• all the tests of errors like long comment, empty string etc should be part of CommentRequestRt tests in x-pack/plugins/cases/common/api/cases/comment/index.test.ts
• where as addComment, update and bulk create just tests the integration with CommentRequestRt with one test
• and same with api integration tests to test route and post_comment, patch_comment, bulk_create_attachments integration with one test?

[adcoelho]

what is the difference between updateComment and actionComment?

Edit: I know it is the type 😅 but more specifically, how are they different in practice?

[js-jankisalvi]

I think @cnasikas can answer it 😄

[cnasikas]

If a user isolates a host from the alerts flyout the security solution team adds a comment to a case + some information about the host etc. At the time we did not have the attachment framework so this became a core attachment type.

[cnasikas]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 3ca8625

Metrics [docs]

Page load bundle

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

id	before	after	diff
cases	145.5KB	145.8KB	+347.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
enterpriseSearch	14	16	+2
securitySolution	410	414	+4
total			+6

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	15	17	+2
securitySolution	489	493	+4
total			+6

History

• 💔 Build #140519 failed 75eb73a
• 💔 Build #140348 failed 480adef
• 💚 Build #140105 succeeded f87deb5

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

cc @js-jankisalvi