action: always direct message commit author on status event regardles…

…s of the branch
ahrefs · Feb 14, 2024 · 6a4ac5a · 6a4ac5a
1 parent e6b2544
commit 6a4ac5a
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 20 deletions.
diff --git a/documentation/config_docs.md b/documentation/config_docs.md
@@ -163,7 +163,7 @@ The following takes place when a status notification is received.
    - `failure`: `allow`
    - `error`: `allow`
    - `success`: `allow_once`
-1. For those payloads allowed by step 1, if it isn't a main branch build notification, query for a slack profile that matches the author of the commit and direct message them. If no profile is found, route to default channel to reduce spam in topic channels. Otherwise, check the notification commit's files according to the prefix rules.
+1. For those payloads allowed by step 1, if it isn't a main branch build notification, route to the default channel to reduce spam in topic channels. Otherwise, check the notification commit's files according to the prefix rules. In addition, query for a slack profile that matches the author of the commit and direct message them.
 
 Internally, the bot keeps track of the status of the last allowed payload, for a given pipeline and branch. This information is used to evaluate the status rules (see below).
 
@@ -175,6 +175,7 @@ Internally, the bot keeps track of the status of the last allowed payload, for a
         "default",
         "buildkite/monorobot-test"
     ],
+    "enable_direct_message": true,
     "rules": [
         {
             "on": ["failure"],
@@ -196,6 +197,7 @@ Internally, the bot keeps track of the status of the last allowed payload, for a
 | value | description | default |
 |-|-|-|
 | `allowed_pipelines` | a list of pipeline names; if specified, payloads whose pipeline name is not in the list will be ignored immediately, without checking the **status rules** | all pipelines included in the status rule check |
+| `enable_direct_message` | control direct message sent to notify about build status | false |
 | `rules` | a list of **status rules** to determine whether to *allow* or *ignore* a payload for further processing | required field |
 
 ### Status Rules

diff --git a/lib/action.ml b/lib/action.ml
@@ -99,30 +99,34 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
     let current_status = n.state in
     let rules = cfg.status_rules.rules in
     let action_on_match (branches : branch list) =
-      let%lwt default =
+      let%lwt direct_message =
         match%lwt Slack_api.lookup_user ~ctx ~cfg ~email:n.commit.commit.author.email with
         (* To send a DM, channel parameter is set to the user id of the recipient *)
         | Ok res -> Lwt.return [ res.user.id ]
         | Error e ->
           log#warn "couldn't match commit email to slack profile: %s" e;
-          Lwt.return (Option.to_list cfg.prefix_rules.default_channel)
+          Lwt.return []
       in
+      let default = Option.to_list cfg.prefix_rules.default_channel in
       let%lwt () = State.set_repo_pipeline_status ctx.state repo.url ~pipeline ~branches ~status:current_status in
-      match List.is_empty branches with
-      | true -> Lwt.return []
-      | false ->
-      match cfg.main_branch_name with
-      | None -> Lwt.return default
-      | Some main_branch_name ->
-      (* non-main branch build notifications go to default channel to reduce spam in topic channels *)
-      match List.exists branches ~f:(fun { name } -> String.equal name main_branch_name) with
-      | false -> Lwt.return default
-      | true ->
-        let sha = n.commit.sha in
-        ( match%lwt Github_api.get_api_commit ~ctx ~repo ~sha with
-        | Error e -> action_error e
-        | Ok commit -> Lwt.return @@ partition_commit cfg commit.files
-        )
+      let%lwt chans =
+        match List.is_empty branches with
+        | true -> Lwt.return []
+        | false ->
+        match cfg.main_branch_name with
+        | None -> Lwt.return default
+        | Some main_branch_name ->
+        (* non-main branch build notifications go to default channel to reduce spam in topic channels *)
+        match List.exists branches ~f:(fun { name } -> String.equal name main_branch_name) with
+        | false -> Lwt.return default
+        | true ->
+          let sha = n.commit.sha in
+          ( match%lwt Github_api.get_api_commit ~ctx ~repo ~sha with
+          | Error e -> action_error e
+          | Ok commit -> Lwt.return @@ partition_commit cfg commit.files
+          )
+      in
+      Lwt.return (direct_message @ chans)
     in
     if Context.is_pipeline_allowed ctx repo.url ~pipeline then begin
       let%lwt repo_state = State.find_or_add_repo ctx.state repo.url in

diff --git a/lib/config.atd b/lib/config.atd
@@ -6,6 +6,7 @@ type project_owners_rule <ocaml from="Rule"> = abstract
 (* This type of rule is used for CI build notifications. *)
 type status_rules = {
   ?allowed_pipelines : string list nullable; (* keep only status events with a title matching this list *)
+  ?enable_direct_message: bool nullable;
   rules: status_rule list;
 }
 
@@ -33,7 +34,7 @@ type project_owners = {
 type config = {
   prefix_rules : prefix_rules;
   label_rules : label_rules;
-  ~status_rules <ocaml default="{allowed_pipelines = Some []; rules = []}"> : status_rules;
+  ~status_rules <ocaml default="{allowed_pipelines = Some []; enable_direct_message = Some false; rules = []}"> : status_rules;
   ~project_owners <ocaml default="{rules = []}"> : project_owners;
   ~ignored_users <ocaml default="[]">: string list; (* list of ignored users *)
   ?main_branch_name : string nullable; (* the name of the main branch; used to filter out notifications about merges of main branch into other branches *)

diff --git a/lib/context.ml b/lib/context.ml
@@ -2,6 +2,8 @@ open Base
 open Common
 open Devkit
 
+let log = Log.from "context"
+
 exception Context_error of string
 
 let context_error fmt = Printf.ksprintf (fun msg -> raise (Context_error msg)) fmt
@@ -69,7 +71,10 @@ let is_pipeline_allowed ctx repo_url ~pipeline =
   | Some allowed_pipelines when not @@ List.exists allowed_pipelines ~f:(String.equal pipeline) -> false
   | _ -> true
 
-let log = Log.from "context"
+let is_status_direct_message_enabled ctx repo_url =
+  match find_repo_config ctx repo_url with
+  | None -> true
+  | Some config -> Option.value config.status_rules.enable_direct_message ~default:false
 
 let refresh_secrets ctx =
   let path = ctx.secrets_filepath in

diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected
@@ -515,9 +515,45 @@ will notify #id[[email protected]]
     }
   ]
 }
+will notify #default
+{
+  "channel": "default",
+  "text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: failure",
+  "attachments": [
+    {
+      "fallback": null,
+      "mrkdwn_in": [ "fields", "text" ],
+      "color": "danger",
+      "text": "*Description*: Build #2 failed (20 seconds).",
+      "fields": [
+        {
+          "value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` Update README.md - Khady\n*Branch*: master"
+        }
+      ]
+    }
+  ]
+}
 ===== file ../mock_payloads/status.merge_develop.json =====
 ===== file ../mock_payloads/status.pending_test.json =====
 ===== file ../mock_payloads/status.state_hide_success_test.json =====
+will notify #id[[email protected]]
+{
+  "channel": "id[[email protected]]",
+  "text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: success",
+  "attachments": [
+    {
+      "fallback": null,
+      "mrkdwn_in": [ "fields", "text" ],
+      "color": "good",
+      "text": "*Description*: Build #2 passed (5 minutes, 19 seconds).",
+      "fields": [
+        {
+          "value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` Update README.md - Khady\n*Branch*: master"
+        }
+      ]
+    }
+  ]
+}
 ===== file ../mock_payloads/status.state_hide_success_test_disallowed_pipeline.json =====
 ===== file ../mock_payloads/status.success_public_repo_no_buildkite.json =====
 will notify #id[[email protected]]
@@ -537,7 +573,42 @@ will notify #id[[email protected]]
     }
   ]
 }
+will notify #default
+{
+  "channel": "default",
+  "text": "<https://github.com/Codertocat/Hello-World|[Codertocat/Hello-World]> CI Build Status notification: success",
+  "attachments": [
+    {
+      "fallback": null,
+      "mrkdwn_in": [ "fields", "text" ],
+      "color": "good",
+      "fields": [
+        {
+          "value": "*Commit*: `<https://github.com/Codertocat/Hello-World/commit/6113728f27ae82c7b1a177c8d03f9e96e0adf246|6113728f>` Initial commit - Codertocat\n*Branches*: master, changes, gh-pages"
+        }
+      ]
+    }
+  ]
+}
 ===== file ../mock_payloads/status.success_test_main_branch.json =====
+will notify #id[[email protected]]
+{
+  "channel": "id[[email protected]]",
+  "text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: success",
+  "attachments": [
+    {
+      "fallback": null,
+      "mrkdwn_in": [ "fields", "text" ],
+      "color": "good",
+      "text": "*Description*: Build #2 passed (5 minutes, 19 seconds).",
+      "fields": [
+        {
+          "value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` Update README.md - Khady\n*Branch*: develop"
+        }
+      ]
+    }
+  ]
+}
 will notify #all-push-events
 {
   "channel": "all-push-events",
@@ -575,6 +646,24 @@ will notify #id[[email protected]]
     }
   ]
 }
+will notify #default
+{
+  "channel": "default",
+  "text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: success",
+  "attachments": [
+    {
+      "fallback": null,
+      "mrkdwn_in": [ "fields", "text" ],
+      "color": "good",
+      "text": "*Description*: Build #2 passed (5 minutes, 19 seconds).",
+      "fields": [
+        {
+          "value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` Update README.md - Khady\n*Branch*: master"
+        }
+      ]
+    }
+  ]
+}
 ===== file ../mock_slack_events/commits.one_file_modified.json =====
 will unfurl in #C047QTRD1CH
 {