[Cases] Notify assignees when assigned to a case #144391

cnasikas · 2022-11-02T09:36:05Z

Summary

The PR adds the ability to notify users by email when assigned to a case. A user is:

Not notified if he/she assigns themselves
Notified if added as an assignee to a case
Not notified if removed from a case

I did not add integration tests due to the complexity of simulating an email server. I added unit test coverage. If integration test coverage is needed we can add the tests on another PR.

Depends on: #143303
Fixes: #142307

Email screenshot

@shanisagiv1 @lcawl What do you think about the content of the email (see screenshot)?

Testing

Put the following in your kibana.yml:

notifications.connectors.default.email: 'mail-dev'

xpack.actions.preconfigured:
  mail-dev:
    name: preconfigured-email-notification-maildev
    actionTypeId: .email
    config:
      service: other
      from: mlr-test-sink@elastic.co
      host: localhost
      port: 1025
      secure: false
      hasAuth: false

Install maildev: npm install -g maildev
Run maildev: maildev
Open MailDev's web interface (http://0.0.0.0:1080/)
Create a case and assign users. You should see the emails in the MailDev inbox.
Update the assignees of a case. You should see the emails in the MailDev inbox.

Note: If you assign yourself you should not see an email. If you delete an assignee you should not see an email.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

Release notes

Notify users by email when assigned to a case

github-actions · 2022-11-02T09:36:19Z

Documentation preview:

✨ Changed pages

elasticmachine · 2022-11-07T11:00:08Z

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

elasticmachine · 2022-11-07T11:00:09Z

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

shanisagiv1 · 2022-11-07T12:29:33Z

a few comments:

can we call it "Elastic Kibana Case" instead of "Elastic Case"
a question for @lcawl - "you got assigned" or "you have been assigned to" - what is more accurate?
"view case" URL --> "View the case details"

not related to the content - how the FROM address is defined?

cnasikas · 2022-11-07T12:33:10Z

can we call it "Elastic Kibana Case" instead of "Elastic Case"

Ofc!

a question for @lcawl - "you got assigned" or "you have been assigned to" - what is more accurate?

This is my mistake. @lcawl suggested, "You are assigned to a case". I forgot to update.

"view case" URL --> "View the case details"

Ok!

not related to the content - how the FROM address is defined?

From the email connector configuration. The user decides what the FROM address will be.

cnasikas · 2022-11-07T15:42:32Z

@shanisagiv1, @gsoldevila found out that in the cloud the from is set automatically to noreply@alerts.elastic.co. On-prem the users can configure it as they want.

lcawl

Suggest replacing "got" in the message, otherwise text LGTM

x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts

x-pack/plugins/cases/server/services/notifications/email_notification_service.ts

jonathan-buttner

Code looks great, left a few comments. I still need to test it. As for integration tests, it looks like the email connector uses a mock email server to receive its emails. Do you think we could implement the same thing? We can do it as a follow up PR.

jonathan-buttner · 2022-11-07T20:57:00Z

x-pack/plugins/cases/server/client/cases/update.test.ts

+      ]);
+    });
+
+    it('does not notify if the case does not exist', async () => {


Should this test ensure that the clientArgs.services.notificationService.bulkNotifyAssignees function was not called?

jonathan-buttner · 2022-11-07T20:57:16Z

x-pack/plugins/cases/server/client/cases/update.test.ts

+      );
+    });
+
+    it('does not notify if the case is patched with the same assignee', async () => {


Should this test ensure that the clientArgs.services.notificationService.bulkNotifyAssignees function was not called?

jonathan-buttner · 2022-11-07T21:21:00Z

x-pack/plugins/cases/server/client/cases/update.ts

+      user
+    );
+
+    if (casesAndAssigneesToNotifyForAssignment.length > 0) {


How about we move the if check inside of the bulkNotifyAssignees and just have it return if the passed in value is an empty array?

jonathan-buttner · 2022-11-07T21:25:33Z

x-pack/plugins/cases/server/client/cases/update.ts

+    }
+
+    return acc;
+  }, [] as Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }>);


nit: I think we can avoid the cast if we add this definition to the reduce<Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }>>

jonathan-buttner · 2022-11-07T21:31:04Z

x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts

+  const caseSO = mockCases[0];
+  const assignees = userProfiles.map((userProfile) => ({ uid: userProfile.uid }));
+
+  const notifications = {


Just curious, does the notification plugin return a mock that we could leverage here?

Good catch! When I wrote the code they did not but they added it later. I will update it to include it.

jonathan-buttner · 2022-11-07T21:34:35Z

x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts

+    ]);
+
+    await emailNotificationService.notifyAssignees({
+      assignees: [...assignees, { uid: assignees[0].uid }],


nit: to make it clearer maybe we should remove the duplicate assignee?

jonathan-buttner · 2022-11-07T21:36:31Z

x-pack/plugins/cases/server/services/notifications/email_notification_service.ts

+    this.publicBaseUrl = publicBaseUrl;
+  }
+
+  private getTitle(theCase: CaseSavedObject) {


nit: we could make this a private static getTitle method.

jonathan-buttner · 2022-11-07T21:38:38Z

x-pack/plugins/cases/server/services/notifications/email_notification_service.ts

+  }
+
+  public async notifyAssignees({ assignees, theCase }: NotifyArgs) {
+    if (!this.notifications.isEmailServiceAvailable()) {


Don't know if it's possible for isEmailServiceAvailable() to throw an error but maybe we should move it into the try?

jonathan-buttner · 2022-11-07T21:40:51Z

x-pack/plugins/cases/server/services/notifications/email_notification_service.ts

+
+  public async bulkNotifyAssignees(args: NotifyArgs[]) {
+    await Promise.all(
+      args.map(({ assignees, theCase }) => this.notifyAssignees({ assignees, theCase }))


Let's use the pMap to ensure we don't make to many concurrent requests.

cnasikas · 2022-11-08T08:12:33Z

Code looks great, left a few comments. I still need to test it. As for integration tests, it looks like the email connector uses a mock email server to receive its emails. Do you think we could implement the same thing? We can do it as a follow up PR.

Thank you @jonathan-buttner for the feedback. I addressed all the suggestions. The email server is not mocked. The service: "__json" is passed as a param when the connector is executed. That is a special "service" that does not actually send the email but it uses nodemailer's jsonTransport to dry run sending emails. In our case, it is not possible to get the output of the executor as it is run in the task manager and the response is not written to any log. We talked about it with @pmuellr and to do it we need to use maildev to mock the email server. Then we need to check for new emails every 1s and save them in memory to be able to later search them when needed from a test. Given the limited time and the amount of work is need to simulate the email server I would prefer to do it on another PR if you do not mind.

jonathan-buttner

Great work! Tested and looks good.

kibana-ci · 2022-11-08T18:18:35Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: c97cd5b

Failed CI Steps

FTR Configs #37

Test Failures

[job] [logs] FTR Configs #37 / Resolver test app when the user is interacting with the node with ID: firstChild when the user hovers over the primary button when the user has clicked the primary button (which selects the node.) should render as expected
[job] [logs] FTR Configs #37 / Resolver test app when the user is interacting with the node with ID: firstChild when the user hovers over the primary button when the user has clicked the primary button (which selects the node.) when the user has moved their mouse off of the primary button (and onto the zoom controls.) should render as expected

Metrics [docs]

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`	108	113	+5
`securitySolution`	440	446	+6
total			+19

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	67	73	+6
`osquery`	109	115	+6
`securitySolution`	517	523	+6
total			+20

History

💚 Build #86340 succeeded 20c4178
💔 Build #86086 failed 0e7d652
💚 Build #85765 succeeded 85c20b0
💔 Build #85747 failed 7d70ad0
💚 Build #85454 succeeded 2458b2f
💚 Build #84579 succeeded 0f90bb7

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

cc @cnasikas

EricDavisX · 2022-11-16T02:03:27Z

Wanted to share that I have done a quick test and seen the 'Cases Assignment will send an email notification' feature working via Cloud-QA deploy of latest 8.6 snapshot code, with the current known requirement that the users that shall receive email notifications are whitelisted in cloud, per this meaning

cnasikas added backport:skip This commit does not require backporting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature release_note:feature Makes this part of the condensed release notes v8.6.0 labels Nov 2, 2022

cnasikas self-assigned this Nov 2, 2022

cnasikas added 7 commits November 4, 2022 13:34

Notify assignees when creating a case

f693c84

Notify assignees when updating a case

103185d

Bulk notify

c56dcff

Improvements

8db6b20

Add backlink to email

6628271

Add unit tests

a9f8098

Fix namespace

2458b2f

cnasikas force-pushed the notify_assignees branch from 0f90bb7 to 2458b2f Compare November 4, 2022 13:16

Merge branch 'main' into notify_assignees

17d82d5

cnasikas marked this pull request as ready for review November 7, 2022 11:00

cnasikas requested a review from a team as a code owner November 7, 2022 11:00

Improvements

85c20b0

cnasikas force-pushed the notify_assignees branch from 7d70ad0 to 85c20b0 Compare November 7, 2022 11:45

lcawl approved these changes Nov 7, 2022

View reviewed changes

lcawl added the ui-copy Review of UI copy with docs team is recommended label Nov 7, 2022

jonathan-buttner reviewed Nov 7, 2022

View reviewed changes

cnasikas added 2 commits November 8, 2022 09:59

PR feedback

ddd148d

Merge branch 'main' into notify_assignees

0e7d652

cnasikas requested a review from jonathan-buttner November 8, 2022 08:12

jonathan-buttner approved these changes Nov 8, 2022

View reviewed changes

cnasikas added 4 commits November 8, 2022 17:56

Fix bug

20c4178

Improve email body

88fd296

Merge branch 'main' into notify_assignees

540e4b4

Add spaceID

c97cd5b

cnasikas enabled auto-merge (squash) November 8, 2022 17:04

cnasikas merged commit 82083c4 into elastic:main Nov 8, 2022

cnasikas deleted the notify_assignees branch November 8, 2022 18:18

milanage mentioned this pull request Jan 22, 2023

[DOCS] Notify assignees when assigned to a case #147713

Merged

This was referenced Mar 7, 2023

[Response Ops][Actions] Keep track of action execution source and show in connector exec log #152030

Merged

[Response Ops][Cases] Add functional tests for notifying assignee when assigned to case #152950

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Notify assignees when assigned to a case #144391

[Cases] Notify assignees when assigned to a case #144391

cnasikas commented Nov 2, 2022 •

edited

Loading

github-actions bot commented Nov 2, 2022

elasticmachine commented Nov 7, 2022

elasticmachine commented Nov 7, 2022

shanisagiv1 commented Nov 7, 2022

cnasikas commented Nov 7, 2022 •

edited

Loading

cnasikas commented Nov 7, 2022

lcawl left a comment

jonathan-buttner left a comment

jonathan-buttner Nov 7, 2022

jonathan-buttner Nov 7, 2022

jonathan-buttner Nov 7, 2022

jonathan-buttner Nov 7, 2022

jonathan-buttner Nov 7, 2022

cnasikas Nov 8, 2022 •

edited

Loading

jonathan-buttner Nov 7, 2022

jonathan-buttner Nov 7, 2022

jonathan-buttner Nov 7, 2022

jonathan-buttner Nov 7, 2022

cnasikas commented Nov 8, 2022

jonathan-buttner left a comment

kibana-ci commented Nov 8, 2022

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

EricDavisX commented Nov 16, 2022

[Cases] Notify assignees when assigned to a case #144391

[Cases] Notify assignees when assigned to a case #144391

Conversation

cnasikas commented Nov 2, 2022 • edited Loading

Summary

Email screenshot

Testing

Checklist

For maintainers

Release notes

github-actions bot commented Nov 2, 2022

elasticmachine commented Nov 7, 2022

elasticmachine commented Nov 7, 2022

shanisagiv1 commented Nov 7, 2022

cnasikas commented Nov 7, 2022 • edited Loading

cnasikas commented Nov 7, 2022

lcawl left a comment

Choose a reason for hiding this comment

jonathan-buttner left a 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

cnasikas Nov 8, 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

cnasikas commented Nov 8, 2022

jonathan-buttner left a comment

Choose a reason for hiding this comment

kibana-ci commented Nov 8, 2022

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

EricDavisX commented Nov 16, 2022

cnasikas commented Nov 2, 2022 •

edited

Loading

cnasikas commented Nov 7, 2022 •

edited

Loading

cnasikas Nov 8, 2022 •

edited

Loading