Skip to content

[Cases] Limit the number of alerts that can be attached to a case#129988

Merged
cnasikas merged 7 commits intoelastic:mainfrom
cnasikas:attach_limits
Apr 14, 2022
Merged

[Cases] Limit the number of alerts that can be attached to a case#129988
cnasikas merged 7 commits intoelastic:mainfrom
cnasikas:attach_limits

Conversation

@cnasikas
Copy link
Copy Markdown
Member

@cnasikas cnasikas commented Apr 12, 2022

Summary

This PR adds a limit to the maximum number of alerts that can be attached to a case. The limit is set to 1K and is applied to a) the total number of alerts attached to a case and b) the total number of alerts that can be saved in the doc of the cases-comments saved object.

Depends on #129092

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cnasikas cnasikas added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// Feature:Cases Cases feature v8.3.0 labels Apr 12, 2022
@cnasikas cnasikas self-assigned this Apr 12, 2022
@cnasikas cnasikas marked this pull request as ready for review April 14, 2022 08:01
@cnasikas cnasikas requested review from a team as code owners April 14, 2022 08:01
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

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

@cnasikas cnasikas requested a review from academo April 14, 2022 08:01
Comment thread x-pack/plugins/cases/server/common/models/case_with_comments.ts
Comment thread x-pack/plugins/cases/server/common/models/case_with_comments.ts
Copy link
Copy Markdown
Contributor

@academo academo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Can you add the following tests?:

  • Fail when the case already has 1k alerts or more
  • Fail when the case already has alerts and the sum of existing and new cases exceed 1k

@cnasikas
Copy link
Copy Markdown
Member Author

This looks good! Can you add the following tests?:

  • Fail when the case already has 1k alerts or more
  • Fail when the case already has alerts and the sum of existing and new cases exceed 1k

Thanks! I don't understand the second bullet. The limit is per case not between all cases.

@cnasikas cnasikas requested a review from academo April 14, 2022 11:25
@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
cases 58 57 -1

History

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

cc @cnasikas

Copy link
Copy Markdown
Contributor

@academo academo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the extra tests!

@cnasikas cnasikas merged commit f5bb693 into elastic:main Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants