Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cases] Show "removed comment" user action in the UI #123352

Merged
merged 3 commits into from
Jan 20, 2022

Conversation

cnasikas
Copy link
Member

@cnasikas cnasikas commented Jan 19, 2022

Summary

At the moment a user cannot delete a comment through the UI. It can be done though from the backend. This PR introduces a "removed comment" action on the comments list UI. Providing the ability to delete comments from the UI should be done in a different PR.

Screenshot 2022-01-19 at 12 44 25 PM

Screenshot 2022-01-19 at 12 45 00 PM

Testing

  1. Create a case
  2. Add comments
  3. Delete a comment with the following API call DELETE https://localhost:5601/api/cases/<case_id>/comments/<comment_id>

Fixes: #98793

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cnasikas 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.1.0 labels Jan 19, 2022
@cnasikas cnasikas self-assigned this Jan 19, 2022
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

FrankHassanabad pushed a commit that referenced this pull request Jan 20, 2022
## Summary

New ECS FieldMap was generated in #123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change.

See #122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528).


Some failing PR's from `main`:
#123357
#121644
#123352

### 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
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 20, 2022
…123429)

## Summary

New ECS FieldMap was generated in elastic#123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change.

See elastic#122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528).

Some failing PR's from `main`:
elastic#123357
elastic#121644
elastic#123352

### 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

(cherry picked from commit d6917fc)
kibanamachine added a commit that referenced this pull request Jan 20, 2022
…#123433)

## Summary

New ECS FieldMap was generated in #123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change.

See #122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528).

Some failing PR's from `main`:
#123357
#121644
#123352

### 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

(cherry picked from commit d6917fc)

Co-authored-by: Garrett Spong <[email protected]>
@spong
Copy link
Member

spong commented Jan 20, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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 311.3KB 311.6KB +239.0B

History

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

cc @cnasikas

@cnasikas cnasikas changed the title [Cases] Show delete comment user action in the UI [Cases] Show "delete comment" user action in the UI Jan 20, 2022
@academo academo changed the title [Cases] Show "delete comment" user action in the UI [Cases] Show "removed comment" user action in the UI Jan 20, 2022
@academo academo self-requested a review January 20, 2022 08:56
@@ -17,6 +17,24 @@ import { createAlertAttachmentUserActionBuilder } from './alert';
import { createActionAttachmentUserActionBuilder } from './actions';

const getUpdateLabelTitle = () => `${i18n.EDITED_FIELD} ${i18n.COMMENT.toLowerCase()}`;
const getDeleteLabelTitle = () => `${i18n.REMOVED_FIELD} ${i18n.COMMENT.toLowerCase()}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: considering you will unlikely reuse this function, you can simply inline the string concatenation in the line 28 where you use it.

@cnasikas cnasikas merged commit b3455bb into elastic:main Jan 20, 2022
@cnasikas cnasikas deleted the delete_comment_user_action branch January 20, 2022 09:02
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 20, 2022
ogupte pushed a commit to ogupte/kibana that referenced this pull request Jan 28, 2022
…123429)

## Summary

New ECS FieldMap was generated in elastic#123012, however since it only contained changes to `Rule Registry` code the `Security Solution` Cypress tests were not run, and thus did not catch this field change.

See elastic#122661 (comment) for details. Confirmed w/ @madirey that expected value is indeed `5` now that `host.geo.continent_code` has been [added](https://github.com/elastic/kibana/pull/123012/files#diff-a1647ccb73ef26c8c8b6aefd87084504b146af72fcb088ccacad93fcaad15b69R1524-R1528).


Some failing PR's from `main`:
elastic#123357
elastic#121644
elastic#123352

### 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
ogupte pushed a commit to ogupte/kibana that referenced this pull request Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting bug Fixes for quality problems that affect the customer experience Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Solution] [Cases] No UI for Deleted Comments
6 participants