diff --git a/lib/action.ml b/lib/action.ml index 9cc90492..c9cecb14 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -58,12 +58,12 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct let partition_pr_review_comment cfg (n : pr_review_comment_notification) = match n.action with - | Created -> partition_label cfg n.pull_request.labels + | Created | Edited -> partition_label cfg n.pull_request.labels | _ -> [] let partition_issue_comment cfg (n : issue_comment_notification) = match n.action with - | Created -> partition_label cfg n.issue.labels + | Created | Edited -> partition_label cfg n.issue.labels | _ -> [] let partition_pr_review cfg (n : pr_review_notification) = @@ -79,7 +79,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct in both cases, since pull_request_review_comment is already handled by another type of event, information in the pull_request_review payload does not provide any insightful information and will thus be ignored. *) - | Submitted, _, _ -> partition_label cfg n.pull_request.labels + | (Submitted | Edited), _, _ -> partition_label cfg n.pull_request.labels | _ -> [] let partition_commit (cfg : Config_t.config) files = @@ -174,6 +174,46 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct | Some sender_login -> List.exists cfg.ignored_users ~f:(String.equal sender_login) | None -> false + let generate_stateful_notification (ctx : Context.t) content req channel = + match req with + | Github.PR_review_comment n -> + ( match n.action with + | Github_t.Created -> Some (generate_new_message content channel) + | Edited -> + ( match State.get_comment_msg ctx.state n.repository.url ~issue_num:n.pull_request.number n.comment.id with + | None -> + log#warn "could not find comment %d in %s for PR #%n" n.comment.id n.repository.url n.pull_request.number; + None + | Some ts -> Some (generate_update_message content ~ts channel) + ) + | _ -> None + ) + | Issue_comment n -> + ( match n.action with + | Github_t.Created -> Some (generate_new_message content channel) + | Edited -> + ( match State.get_comment_msg ctx.state n.repository.url ~issue_num:n.issue.number n.comment.id with + | None -> + log#warn "could not find comment %d in %s for issue #%n" n.comment.id n.repository.url n.issue.number; + None + | Some ts -> Some (generate_update_message content ~ts channel) + ) + | _ -> None + ) + | PR_review n -> + ( match n.action with + | Github_t.Submitted -> Some (generate_new_message content channel) + | Edited -> + ( match State.get_review_msg ctx.state n.repository.url ~issue_num:n.pull_request.number n.review.id with + | None -> + log#warn "could not find review %d in %s for PR #%n" n.review.id n.repository.url n.pull_request.number; + None + | Some ts -> Some (generate_update_message content ~ts channel) + ) + | _ -> None + ) + | _ -> Some (generate_new_message content channel) + let generate_notifications (ctx : Context.t) (req : Github.t) = let repo = Github.repo_of_notification req in let cfg = Context.find_repo_config_exn ctx repo.url in @@ -182,30 +222,84 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct | false -> match req with | Github.Push n -> - partition_push cfg n |> List.map ~f:(fun (channel, n) -> generate_push_notification n channel) |> Lwt.return - | Pull_request n -> partition_pr cfg n |> List.map ~f:(generate_pull_request_notification n) |> Lwt.return - | PR_review n -> partition_pr_review cfg n |> List.map ~f:(generate_pr_review_notification n) |> Lwt.return + partition_push cfg n + |> List.map ~f:(fun (channel, n) -> + generate_stateful_notification ctx (generate_push_notification_content n) req channel + ) + |> Lwt.return + | Pull_request n -> + partition_pr cfg n + |> List.map ~f:(fun channel -> + generate_stateful_notification ctx (generate_pull_request_notification_content n) req channel + ) + |> Lwt.return + | PR_review n -> + partition_pr_review cfg n + |> List.map ~f:(fun channel -> + generate_stateful_notification ctx (generate_pr_review_notification_content n) req channel + ) + |> Lwt.return | PR_review_comment n -> - partition_pr_review_comment cfg n |> List.map ~f:(generate_pr_review_comment_notification n) |> Lwt.return - | Issue n -> partition_issue cfg n |> List.map ~f:(generate_issue_notification n) |> Lwt.return + partition_pr_review_comment cfg n + |> List.map ~f:(fun channel -> + generate_stateful_notification ctx (generate_pr_review_comment_notification_content n) req channel + ) + |> Lwt.return + | Issue n -> + partition_issue cfg n + |> List.map ~f:(fun channel -> + generate_stateful_notification ctx (generate_issue_notification_content n) req channel + ) + |> Lwt.return | Issue_comment n -> - partition_issue_comment cfg n |> List.map ~f:(generate_issue_comment_notification n) |> Lwt.return + partition_issue_comment cfg n + |> List.map ~f:(fun channel -> + (* not sure why but when I remove "fun channel" it evaluates the [generate_issue_comment_notification_content n] before the partition so I got errors but when I add this, it's completely fine, probably some compiler optimization? *) + generate_stateful_notification ctx (generate_issue_comment_notification_content n) req channel + ) + |> Lwt.return | Commit_comment n -> let%lwt channels, api_commit = partition_commit_comment ctx n in - let notifs = List.map ~f:(generate_commit_comment_notification api_commit n) channels in + let notifs = + List.map + ~f:(generate_stateful_notification ctx (generate_commit_comment_notification_content api_commit n) req) + channels + in Lwt.return notifs | Status n -> let%lwt channels = partition_status ctx n in - let notifs = List.map ~f:(generate_status_notification cfg n) channels in + let notifs = + List.map + ~f:(fun channel -> generate_stateful_notification ctx (generate_status_notification_content cfg n) req channel) + channels + in Lwt.return notifs | _ -> Lwt.return [] - let send_notifications (ctx : Context.t) notifications = - let notify (msg : Slack_t.post_message_req) = - match%lwt Slack_api.send_notification ~ctx ~msg with - | Ok () -> Lwt.return_unit - | Error e -> action_error e + let send_notifications (ctx : Context.t) (req : Github.t) notifications = + let update_comment_mapping message = + match req with + | Github.PR_review_comment n -> + State.add_comment_msg ctx.state n.repository.url ~issue_num:n.pull_request.number n.comment.id message + | Issue_comment n -> + State.add_comment_msg ctx.state n.repository.url ~issue_num:n.issue.number n.comment.id message + | PR_review n -> + State.add_review_msg ctx.state n.repository.url ~issue_num:n.pull_request.number n.review.id message + | _ -> Lwt.return_unit + in + let notify = function + | New_message (msg : Slack_t.post_message_req) -> + ( match%lwt Slack_api.send_notification ~ctx ~msg with + | Ok res -> update_comment_mapping res.ts + | Error e -> action_error e + ) + | Update_message msg -> + ( match%lwt Slack_api.update_notification ~ctx ~msg with + | Ok () -> Lwt.return_unit + | Error e -> action_error e + ) in + Lwt_list.iter_s notify notifications (** [refresh_repo_config ctx n] fetches the latest repo config if it's @@ -253,6 +347,13 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct end | _ -> Lwt.return_unit + let cleanup_state (ctx : Context.t) (payload : Github.t) = + match payload with + | Github.Pull_request { action = Closed; pull_request = { number; _ }; repository = { url; _ }; _ } + | Issue { action = Closed; issue = { number; _ }; repository = { url; _ }; _ } -> + State.close_issue ctx.state url number + | _ -> Lwt.return_unit + let process_github_notification (ctx : Context.t) headers body = let validate_signature secrets payload = let repo = Github.repo_of_notification payload in @@ -278,10 +379,14 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct | Error e -> action_error e | Ok () -> let%lwt notifications = generate_notifications ctx payload in - let%lwt () = Lwt.join [ send_notifications ctx notifications; do_github_tasks ctx repo payload ] in + let%lwt () = + Lwt.join + [ send_notifications ctx payload (List.filter_opt notifications); do_github_tasks ctx repo payload ] + in ( match ctx.state_filepath with | None -> Lwt.return_unit | Some path -> + let%lwt () = cleanup_state ctx payload in ( match%lwt State.save ctx.state path with | Ok () -> Lwt.return_unit | Error e -> action_error e diff --git a/lib/api.ml b/lib/api.ml index c1be62a0..a01cf91b 100644 --- a/lib/api.ml +++ b/lib/api.ml @@ -18,7 +18,8 @@ module type Github = sig end module type Slack = sig - val send_notification : ctx:Context.t -> msg:post_message_req -> unit slack_response Lwt.t + val send_notification : ctx:Context.t -> msg:post_message_req -> post_message_res slack_response Lwt.t + val update_notification : ctx:Context.t -> msg:update_message_req -> unit slack_response Lwt.t val send_chat_unfurl : ctx:Context.t -> diff --git a/lib/api_local.ml b/lib/api_local.ml index 524c241d..2319bec6 100644 --- a/lib/api_local.ml +++ b/lib/api_local.ml @@ -53,6 +53,7 @@ end (** The base implementation for local check payload debugging and mocking tests *) module Slack_base : Api.Slack = struct let send_notification ~ctx:_ ~msg:_ = Lwt.return @@ Error "undefined for local setup" + let update_notification ~ctx:_ ~msg:_ = Lwt.return @@ Error "undefined for local setup" let send_chat_unfurl ~ctx:_ ~channel:_ ~ts:_ ~unfurls:_ () = Lwt.return @@ Error "undefined for local setup" let send_auth_test ~ctx:_ () = Lwt.return @@ Error "undefined for local setup" end @@ -65,6 +66,14 @@ module Slack : Api.Slack = struct let json = msg |> Slack_j.string_of_post_message_req |> Yojson.Basic.from_string |> Yojson.Basic.pretty_to_string in Stdio.printf "will notify #%s\n" msg.channel; Stdio.printf "%s\n" json; + Lwt.return @@ Ok ({ channel = msg.channel; ts = "SOMETS" } : Slack_t.post_message_res) + + let update_notification ~ctx:_ ~msg = + let json = + msg |> Slack_j.string_of_update_message_req |> Yojson.Basic.from_string |> Yojson.Basic.pretty_to_string + in + Stdio.printf "will update msg in #%s\n" msg.channel; + Stdio.printf "%s\n" json; Lwt.return @@ Ok () let send_chat_unfurl ~ctx:_ ~channel ~ts ~unfurls () = @@ -91,6 +100,14 @@ module Slack_simple : Api.Slack = struct | None -> "" | Some s -> sprintf " with %S" s ); + Lwt.return @@ Ok ({ channel = msg.channel; ts = "SOMETS" } : Slack_t.post_message_res) + + let update_notification ~ctx:_ ~(msg : Slack_t.update_message_req) = + log#info "will update msg in #%s: %s\n" msg.channel + ( match msg.Slack_t.text with + | None -> "" + | Some s -> sprintf " with %S" s + ); Lwt.return @@ Ok () let send_chat_unfurl ~ctx:_ ~channel ~ts:_ ~(unfurls : Slack_t.message_attachment Common.StringMap.t) () = @@ -117,6 +134,15 @@ module Slack_json : Api.Slack = struct let url = Uri.add_query_param url ("msg", [ json ]) in log#info "%s" (Uri.to_string url); log#info "%s" json; + Lwt.return @@ Ok ({ channel = msg.channel; ts = "SOMETS" } : Slack_t.post_message_res) + + let update_notification ~ctx:_ ~(msg : Slack_t.update_message_req) = + log#info "will notify %s" msg.channel; + let json = Slack_j.string_of_update_message_req msg in + let url = Uri.of_string "https://api.slack.com/docs/messages/builder" in + let url = Uri.add_query_param url ("msg", [ json ]) in + log#info "%s" (Uri.to_string url); + log#info "%s" json; Lwt.return @@ Ok () let send_chat_unfurl ~ctx:_ ~channel ~ts:_ ~(unfurls : Slack_t.message_attachment Common.StringMap.t) () = diff --git a/lib/api_remote.ml b/lib/api_remote.ml index 525d527f..0aba78e2 100644 --- a/lib/api_remote.ml +++ b/lib/api_remote.ml @@ -117,36 +117,46 @@ module Slack : Api.Slack = struct (* must read whole response to update lexer state *) ignore (Slack_j.read_ok_res s l) + let webhook_channel_request (ctx : Context.t) ~channel ~build_error read body = + match Context.hook_of_channel ctx channel with + | Some url -> + ( match%lwt http_request ~body ~headers:[] `POST url with + | Ok res -> Lwt.return_ok @@ read res + | Error e -> Lwt.return @@ build_error (query_error_msg url e) + ) + | None -> Lwt.return @@ build_error (sprintf "no hook for channel: %s" channel) + (** [send_notification ctx msg] notifies [msg.channel] with the payload [msg]; uses web API with access token if available, or with webhook otherwise *) let send_notification ~(ctx : Context.t) ~(msg : Slack_t.post_message_req) = log#info "sending to %s" msg.channel; let build_error e = fmt_error "%s\nfailed to send Slack notification" e in - let secrets = Context.get_secrets_exn ctx in - let headers, url, webhook_mode = - match Context.hook_of_channel ctx msg.channel with - | Some url -> [], Some url, true - | None -> - match secrets.slack_access_token with - | Some access_token -> [ bearer_token_header access_token ], Some "https://slack.com/api/chat.postMessage", false - | None -> [], None, false - in - match url with - | None -> Lwt.return @@ build_error @@ sprintf "no token or webhook configured to notify channel %s" msg.channel - | Some url -> - let data = Slack_j.string_of_post_message_req msg in - let body = `Raw ("application/json", data) in - log#info "data: %s" data; - if webhook_mode then begin - match%lwt http_request ~body ~headers `POST url with - | Ok _res -> Lwt.return @@ Ok () - | Error e -> Lwt.return @@ build_error (query_error_msg url e) - end - else begin - match%lwt slack_api_request ~body ~headers `POST url Slack_j.read_post_message_res with - | Ok _res -> Lwt.return @@ Ok () - | Error e -> Lwt.return @@ build_error e - end + let data = Slack_j.string_of_post_message_req msg in + let body = `Raw ("application/json", data) in + log#info "data: %s" data; + match%lwt webhook_channel_request ctx ~channel:msg.channel ~build_error Slack_j.post_message_res_of_string body with + | Ok res -> Lwt.return_ok res + | Error e -> + log#warn "failed to run webhook call: %s" e; + request_token_auth ~name:"post message to channel" ~body ~ctx `POST "chat.postMessage" + Slack_j.read_post_message_res + + (** [update_notification ctx msg] update a message at [msg.ts] in [msg.channel] + with the payload [msg]; uses web API with access token if available, or with + webhook otherwise *) + let update_notification ~(ctx : Context.t) ~(msg : Slack_t.update_message_req) = + log#info "sending to %s" msg.channel; + let build_error e = fmt_error "%s\nfailed to update Slack notification" e in + let data = Slack_j.string_of_update_message_req msg in + let body = `Raw ("application/json", data) in + log#info "data: %s" data; + match%lwt + webhook_channel_request ctx ~channel:msg.channel ~build_error Slack_j.update_message_res_of_string body + with + | Ok (_res : Slack_t.update_message_res) -> Lwt.return_ok () + | Error e -> + log#warn "failed to run webhook call: %s" e; + request_token_auth ~name:"update message in channel" ~body ~ctx `POST "chat.update" read_unit let send_chat_unfurl ~(ctx : Context.t) ~channel ~ts ~unfurls () = let req = Slack_j.{ channel; ts; unfurls } in diff --git a/lib/github.atd b/lib/github.atd index 43c55fa4..2a424942 100644 --- a/lib/github.atd +++ b/lib/github.atd @@ -185,6 +185,7 @@ type pr_notification = { type review = { ?body: string nullable; + id: int; html_url: string; state: string; } diff --git a/lib/slack.atd b/lib/slack.atd index 285a12b8..a977eef9 100644 --- a/lib/slack.atd +++ b/lib/slack.atd @@ -65,6 +65,17 @@ type post_message_req = { type post_message_res = { channel: string; + ts: string; +} + +type update_message_req = { + inherit post_message_req; + ts: string; +} + +type update_message_res = { + channel: string; + ts: string; } type link_shared_link = { diff --git a/lib/slack.ml b/lib/slack.ml index bb22457e..b67e4c81 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -36,7 +36,7 @@ let show_labels = function let pluralize name num suffix = if num = 1 then sprintf "%s" name else String.concat [ name; suffix ] -let generate_pull_request_notification notification channel = +let generate_pull_request_notification_content notification = let { action; number; sender; pull_request; repository } = notification in let ({ body; title; html_url; labels; merged; _ } : pull_request) = pull_request in let action, body = @@ -55,28 +55,17 @@ let generate_pull_request_notification notification channel = sprintf "<%s|[%s]> Pull request #%d %s %s by *%s*" repository.url repository.full_name number (pp_link ~url:html_url title) action sender.login in - { - channel; - text = Some summary; - attachments = - Some - [ - { - empty_attachments with - mrkdwn_in = Some [ "text" ]; - color = Some "#ccc"; - text = mrkdwn_of_markdown_opt body; - }; - ]; - blocks = None; - } + let attachments = + [ { empty_attachments with mrkdwn_in = Some [ "text" ]; color = Some "#ccc"; text = mrkdwn_of_markdown_opt body } ] + in + attachments, summary -let generate_pr_review_notification notification channel = +let generate_pr_review_notification_content notification = let { action; sender; pull_request; review; repository } = notification in let ({ number; title; html_url; _ } : pull_request) = pull_request in let action_str = match action with - | Submitted -> + | Submitted | Edited -> ( match review.state with | "commented" -> "commented on" | "approved" -> "approved" @@ -93,28 +82,24 @@ let generate_pr_review_notification notification channel = sprintf "<%s|[%s]> *%s* <%s|%s> #%d %s" repository.url repository.full_name sender.login review.html_url action_str number (pp_link ~url:html_url title) in - { - channel; - text = Some summary; - attachments = - Some - [ - { - empty_attachments with - mrkdwn_in = Some [ "text" ]; - color = Some "#ccc"; - text = mrkdwn_of_markdown_opt review.body; - }; - ]; - blocks = None; - } + let attachments = + [ + { + empty_attachments with + mrkdwn_in = Some [ "text" ]; + color = Some "#ccc"; + text = mrkdwn_of_markdown_opt review.body; + }; + ] + in + attachments, summary -let generate_pr_review_comment_notification notification channel = +let generate_pr_review_comment_notification_content notification = let { action; pull_request; sender; comment; repository } = notification in let ({ number; title; html_url; _ } : pull_request) = pull_request in let action_str = match action with - | Created -> "commented" + | Created | Edited -> "commented" | _ -> invalid_arg (sprintf "Monorobot doesn't know how to generate notification for the unexpected event %s" @@ -130,24 +115,20 @@ let generate_pr_review_comment_notification notification channel = | None -> None | Some a -> Some (sprintf "New comment by %s in <%s|%s>" sender.login comment.html_url a) in - { - channel; - text = Some summary; - attachments = - Some - [ - { - empty_attachments with - mrkdwn_in = Some [ "text" ]; - color = Some "#ccc"; - footer = file; - text = Some (mrkdwn_of_markdown comment.body); - }; - ]; - blocks = None; - } + let attachments = + [ + { + empty_attachments with + mrkdwn_in = Some [ "text" ]; + color = Some "#ccc"; + footer = file; + text = Some (mrkdwn_of_markdown comment.body); + }; + ] + in + attachments, summary -let generate_issue_notification notification channel = +let generate_issue_notification_content notification = let ({ action; sender; issue; repository } : issue_notification) = notification in let { number; body; title; html_url; labels; _ } = issue in let action, body = @@ -166,28 +147,17 @@ let generate_issue_notification notification channel = sprintf "<%s|[%s]> Issue #%d %s %s by *%s*" repository.url repository.full_name number (pp_link ~url:html_url title) action sender.login in - { - channel; - text = Some summary; - attachments = - Some - [ - { - empty_attachments with - mrkdwn_in = Some [ "text" ]; - color = Some "#ccc"; - text = mrkdwn_of_markdown_opt body; - }; - ]; - blocks = None; - } + let attachments = + [ { empty_attachments with mrkdwn_in = Some [ "text" ]; color = Some "#ccc"; text = mrkdwn_of_markdown_opt body } ] + in + attachments, summary -let generate_issue_comment_notification notification channel = +let generate_issue_comment_notification_content notification = let { action; issue; sender; comment; repository } = notification in let { number; title; _ } = issue in let action_str = match action with - | Created -> "commented" + | Created | Edited -> "commented" | _ -> invalid_arg (sprintf @@ -199,21 +169,17 @@ let generate_issue_comment_notification notification channel = sprintf "<%s|[%s]> *%s* <%s|%s> on #%d %s" repository.url repository.full_name sender.login comment.html_url action_str number (pp_link ~url:issue.html_url title) in - { - channel; - text = Some summary; - attachments = - Some - [ - { - empty_attachments with - mrkdwn_in = Some [ "text" ]; - color = Some "#ccc"; - text = Some (mrkdwn_of_markdown comment.body); - }; - ]; - blocks = None; - } + let attachments = + [ + { + empty_attachments with + mrkdwn_in = Some [ "text" ]; + color = Some "#ccc"; + text = Some (mrkdwn_of_markdown comment.body); + }; + ] + in + attachments, summary let git_short_sha_hash hash = String.sub ~pos:0 ~len:8 hash @@ -245,7 +211,7 @@ let pp_list_with_previews ~pp_item list = end else List.map ~f:pp_item list -let generate_push_notification notification channel = +let generate_push_notification_content notification = let { sender; created; deleted; forced; compare; commits; repository; _ } = notification in let commits_branch = Github.commits_branch_of_ref notification.ref in let tree_url = String.concat ~sep:"/" [ repository.url; "tree"; Uri.pct_encode commits_branch ] in @@ -271,23 +237,19 @@ let generate_push_notification notification channel = sender.login in let commits = pp_list_with_previews ~pp_item:pp_commit commits in - { - channel; - text = Some title; - attachments = - Some - [ - { - empty_attachments with - mrkdwn_in = Some [ "fields" ]; - color = Some "#ccc"; - fields = Some [ { value = String.concat ~sep:"\n" commits; title = None; short = false } ]; - }; - ]; - blocks = None; - } + let attachments = + [ + { + empty_attachments with + mrkdwn_in = Some [ "fields" ]; + color = Some "#ccc"; + fields = Some [ { value = String.concat ~sep:"\n" commits; title = None; short = false } ]; + }; + ] + in + attachments, title -let generate_status_notification (cfg : Config_t.config) (notification : status_notification) channel = +let generate_status_notification_content (cfg : Config_t.config) (notification : status_notification) = let { commit; state; description; target_url; context; repository; _ } = notification in let ({ commit : inner_commit; sha; author; html_url; _ } : status_commit) = commit in let ({ message; _ } : inner_commit) = commit in @@ -338,18 +300,20 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ context state_info in let msg = String.concat ~sep:"\n" @@ List.concat [ commit_info; branches_info ] in - let attachment = - { - empty_attachments with - mrkdwn_in = Some [ "fields"; "text" ]; - color = Some color_info; - text = description_info; - fields = Some [ { title = None; value = msg; short = false } ]; - } + let attachments = + [ + { + empty_attachments with + mrkdwn_in = Some [ "fields"; "text" ]; + color = Some color_info; + text = description_info; + fields = Some [ { title = None; value = msg; short = false } ]; + }; + ] in - { channel; text = Some summary; attachments = Some [ attachment ]; blocks = None } + attachments, summary -let generate_commit_comment_notification api_commit notification channel = +let generate_commit_comment_notification_content api_commit notification = let { commit; _ } = api_commit in let { sender; comment; repository; _ } = notification in let commit_id = @@ -367,16 +331,28 @@ let generate_commit_comment_notification api_commit notification channel = | None -> None | Some p -> Some (sprintf "New comment by %s in <%s|%s>" sender.login comment.html_url p) in - let attachment = - { - empty_attachments with - mrkdwn_in = Some [ "pretext"; "text" ]; - color = Some "#ccc"; - footer = path; - text = Some (mrkdwn_of_markdown comment.body); - } + let attachments = + [ + { + empty_attachments with + mrkdwn_in = Some [ "pretext"; "text" ]; + color = Some "#ccc"; + footer = path; + text = Some (mrkdwn_of_markdown comment.body); + }; + ] in - { channel; text = Some summary; attachments = Some [ attachment ]; blocks = None } + attachments, summary + +type send_notification_mode = + | New_message of Slack_t.post_message_req + | Update_message of Slack_t.update_message_req + +let generate_new_message (attachments, summary) channel = + New_message { channel; text = Some summary; attachments = Some attachments; blocks = None } + +let generate_update_message (attachments, summary) ~ts channel = + Update_message { channel; ts; text = Some summary; attachments = Some attachments; blocks = None } let validate_signature ?(version = "v0") ?signing_key ~headers body = match signing_key with diff --git a/lib/state.atd b/lib/state.atd index 86b18f06..5f73e78a 100644 --- a/lib/state.atd +++ b/lib/state.atd @@ -10,13 +10,21 @@ type branch_statuses = status_state map_as_object branch *) type pipeline_statuses = branch_statuses map_as_object +type ts_map = string map_as_object +type issue_state = { + comments : ts_map; + reviews : ts_map; +} + (* The runtime state of a given GitHub repository *) type repo_state = { - pipeline_statuses : pipeline_statuses + pipeline_statuses : pipeline_statuses; + (* issue number is the same as PR number for github APIs *) + issue_tbl : issue_state table_as_object; } (* The serializable runtime state of the bot *) type state = { repos : repo_state table_as_object; ?bot_user_id : string nullable; -} \ No newline at end of file +} diff --git a/lib/state.ml b/lib/state.ml index 6a53f361..c3e9db7d 100644 --- a/lib/state.ml +++ b/lib/state.ml @@ -7,7 +7,8 @@ type t = { lock : Lwt_mutex.t; (** protect access to mutable string map `pipeline_statuses` *) } -let empty_repo_state () : State_t.repo_state = { pipeline_statuses = StringMap.empty } +let empty_repo_state () : State_t.repo_state = { pipeline_statuses = StringMap.empty; issue_tbl = Stringtbl.empty () } +let empty_issue_state () : State_t.issue_state = { comments = StringMap.empty; reviews = StringMap.empty } let empty () : t = let state = State_t.{ repos = Stringtbl.empty (); bot_user_id = None } in @@ -15,6 +16,9 @@ let empty () : t = let find_or_add_repo' state repo_url = Stringtbl.find_or_add state.State_t.repos repo_url ~default:empty_repo_state +let find_or_add_issue' (repo_state : State_t.repo_state) issue_num = + Stringtbl.find_or_add repo_state.issue_tbl (Int.to_string issue_num) ~default:empty_issue_state + let set_repo_state { state; lock } repo_url repo_state = Lwt_mutex.with_lock lock @@ fun () -> Stringtbl.set state.repos ~key:repo_url ~data:repo_state; @@ -34,6 +38,36 @@ let set_repo_pipeline_status { state; lock } repo_url ~pipeline ~(branches : Git repo_state.pipeline_statuses <- Map.update repo_state.pipeline_statuses pipeline ~f:set_branch_status; Lwt.return_unit +let add_comment_msg { state; lock } repo_url ~issue_num comment_id post_message_res = + Lwt_mutex.with_lock lock @@ fun () -> + let repo_state = find_or_add_repo' state repo_url in + let issue_tbl = find_or_add_issue' repo_state issue_num in + issue_tbl.comments <- Map.set issue_tbl.comments ~key:(Int.to_string comment_id) ~data:post_message_res; + Lwt.return_unit + +let get_comment_msg { state; _ } repo_url ~issue_num comment_id = + let repo_state = find_or_add_repo' state repo_url in + let issue_tbl = find_or_add_issue' repo_state issue_num in + Map.find issue_tbl.comments (Int.to_string comment_id) + +let add_review_msg { state; lock } repo_url ~issue_num review_id post_message_res = + Lwt_mutex.with_lock lock @@ fun () -> + let repo_state = find_or_add_repo' state repo_url in + let issue_tbl = find_or_add_issue' repo_state issue_num in + issue_tbl.reviews <- Map.set issue_tbl.reviews ~key:(Int.to_string review_id) ~data:post_message_res; + Lwt.return_unit + +let get_review_msg { state; _ } repo_url ~issue_num review_id = + let repo_state = find_or_add_repo' state repo_url in + let issue_tbl = find_or_add_issue' repo_state issue_num in + Map.find issue_tbl.reviews (Int.to_string review_id) + +let close_issue { state; lock } repo_url issue_num = + Lwt_mutex.with_lock lock @@ fun () -> + let repo_state = find_or_add_repo' state repo_url in + Stringtbl.remove repo_state.issue_tbl (Int.to_string issue_num); + Lwt.return_unit + let set_bot_user_id { state; _ } user_id = state.State_t.bot_user_id <- Some user_id let get_bot_user_id { state; _ } = state.State_t.bot_user_id let log = Log.from "state" diff --git a/mock_states/issue_comment.edited.json b/mock_states/issue_comment.edited.json new file mode 100644 index 00000000..b127ac8b --- /dev/null +++ b/mock_states/issue_comment.edited.json @@ -0,0 +1,21 @@ + +{ + "pipeline_statuses": { + "default": { + "master": "failure" + }, + "buildkite/pipeline2": { + "master": "success" + } + }, + "issue_tbl" : { + "4" : { + "comments": { + "0": "1.00" + }, + "reviews": { + "0": "2.00" + } + } + } +} diff --git a/mock_states/pull_request_review.edited.json b/mock_states/pull_request_review.edited.json new file mode 100644 index 00000000..7834820b --- /dev/null +++ b/mock_states/pull_request_review.edited.json @@ -0,0 +1,21 @@ + +{ + "pipeline_statuses": { + "default": { + "master": "failure" + }, + "buildkite/pipeline2": { + "master": "success" + } + }, + "issue_tbl" : { + "4": { + "comments": { + "0": "1.00" + }, + "reviews": { + "0": "2.00" + } + } + } +} diff --git a/mock_states/pull_request_review_comment.edited.json b/mock_states/pull_request_review_comment.edited.json new file mode 100644 index 00000000..7834820b --- /dev/null +++ b/mock_states/pull_request_review_comment.edited.json @@ -0,0 +1,21 @@ + +{ + "pipeline_statuses": { + "default": { + "master": "failure" + }, + "buildkite/pipeline2": { + "master": "success" + } + }, + "issue_tbl" : { + "4": { + "comments": { + "0": "1.00" + }, + "reviews": { + "0": "2.00" + } + } + } +} diff --git a/mock_states/status.state_hide_success_test.json b/mock_states/status.state_hide_success_test.json index 0a4b7dec..7834820b 100644 --- a/mock_states/status.state_hide_success_test.json +++ b/mock_states/status.state_hide_success_test.json @@ -1,11 +1,21 @@ { - "pipeline_statuses": { - "default": { - "master": "failure" + "pipeline_statuses": { + "default": { + "master": "failure" + }, + "buildkite/pipeline2": { + "master": "success" + } }, - "buildkite/pipeline2": { - "master": "success" + "issue_tbl" : { + "4": { + "comments": { + "0": "1.00" + }, + "reviews": { + "0": "2.00" + } + } } - } -} \ No newline at end of file +} diff --git a/mock_states/status.state_hide_success_test_disallowed_pipeline.json b/mock_states/status.state_hide_success_test_disallowed_pipeline.json index 25b4592a..f85336ab 100644 --- a/mock_states/status.state_hide_success_test_disallowed_pipeline.json +++ b/mock_states/status.state_hide_success_test_disallowed_pipeline.json @@ -1,7 +1,17 @@ { - "pipeline_statuses": { - "buildkite/pipeline2": { - "master": "failure" + "pipeline_statuses": { + "buildkite/pipeline2": { + "master": "failure" + } + }, + "issue_tbl" : { + "4": { + "comments": { + "0": "1.00" + }, + "reviews": { + "0": "2.00" + } + } } - } -} \ No newline at end of file +} diff --git a/src/monorobot.ml b/src/monorobot.ml index b4aa1394..7fa9e6e6 100644 --- a/src/monorobot.ml +++ b/src/monorobot.ml @@ -57,7 +57,7 @@ let check_slack_action file secrets = | Error e -> log#error "%s" e; Lwt.return_unit - | Ok () -> Lwt.return_unit + | Ok (_res : Slack_t.post_message_res) -> Lwt.return_unit ) (* flags *) diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 1e3c84fe..d8ec7477 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -171,6 +171,34 @@ will notify #frontend-bot ] } ===== file ../mock_payloads/issue_comment.edited.json ===== +will update msg in #a1-bot +{ + "channel": "a1-bot", + "text": " *xinyuluo* on #4 ", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "text" ], + "color": "#ccc", + "text": "issue comment edited test" + } + ], + "ts": "1.00" +} +will update msg in #backend +{ + "channel": "backend", + "text": " *xinyuluo* on #4 ", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "text" ], + "color": "#ccc", + "text": "issue comment edited test" + } + ], + "ts": "1.00" +} ===== file ../mock_payloads/issues.closed.json ===== will notify #backend { @@ -307,6 +335,34 @@ will notify #frontend-bot } ===== file ../mock_payloads/pull_request_review.dismissed.json ===== ===== file ../mock_payloads/pull_request_review.edited.json ===== +will update msg in #a1-bot +{ + "channel": "a1-bot", + "text": " *xinyuluo* #4 ", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "text" ], + "color": "#ccc", + "text": "pr review comment test" + } + ], + "ts": "2.00" +} +will update msg in #backend +{ + "channel": "backend", + "text": " *xinyuluo* #4 ", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "text" ], + "color": "#ccc", + "text": "pr review comment test" + } + ], + "ts": "2.00" +} ===== file ../mock_payloads/pull_request_review.request_changes.json ===== will notify #default { @@ -354,6 +410,36 @@ will notify #backend } ===== file ../mock_payloads/pull_request_review_comment.deleted.json ===== ===== file ../mock_payloads/pull_request_review_comment.edited.json ===== +will update msg in #a1-bot +{ + "channel": "a1-bot", + "text": " *xinyuluo* commented on #4 ", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "text" ], + "color": "#ccc", + "text": "Edited test comment", + "footer": "New comment by xinyuluo in " + } + ], + "ts": "1.00" +} +will update msg in #backend +{ + "channel": "backend", + "text": " *xinyuluo* commented on #4 ", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "text" ], + "color": "#ccc", + "text": "Edited test comment", + "footer": "New comment by xinyuluo in " + } + ], + "ts": "1.00" +} ===== file ../mock_payloads/push.branch_filter_default.json ===== will notify #all-push-events { diff --git a/test/test.ml b/test/test.ml index e7f0df93..d169ad8f 100644 --- a/test/test.ml +++ b/test/test.ml @@ -27,52 +27,60 @@ let get_mock_slack_events () = let process_gh_payload ~(secrets : Config_t.secrets) ~config (kind, path, state_path) = let headers = [ "x-github-event", kind ] in - let make_test_context event = - let repo = Github.repo_of_notification @@ Github.parse_exn headers event in - (* overwrite repo url in secrets with that of notification for this test case *) - let secrets = { secrets with repos = [ { url = repo.url; gh_token = None; gh_hook_token = None } ] } in - let ctx = Context.make () in - ctx.secrets <- Some secrets; - let%lwt _ = State.find_or_add_repo ctx.state repo.url in - match state_path with - | None -> - Context.set_repo_config ctx repo.url config; - Lwt.return ctx - | Some state_path -> - match Common.get_local_file state_path with - | Error e -> - log#error "failed to read %s: %s" state_path e; - Lwt.return ctx - | Ok file -> - let repo_state = State_j.repo_state_of_string file in - let%lwt () = State.set_repo_state ctx.state repo.url repo_state in - Context.set_repo_config ctx repo.url config; - Lwt.return ctx - in - Stdio.printf "===== file %s =====\n" path; - let headers = [ "x-github-event", kind ] in - match Common.get_local_file path with - | Error e -> Lwt.return @@ log#error "failed to read %s: %s" path e - | Ok event -> - let%lwt ctx = make_test_context event in - let%lwt _ctx = Action_local.process_github_notification ctx headers event in - Lwt.return_unit + try%lwt + let make_test_context event = + let repo = Github.repo_of_notification @@ Github.parse_exn headers event in + (* overwrite repo url in secrets with that of notification for this test case *) + let secrets = { secrets with repos = [ { url = repo.url; gh_token = None; gh_hook_token = None } ] } in + let ctx = Context.make () in + ctx.secrets <- Some secrets; + let%lwt _ = State.find_or_add_repo ctx.state repo.url in + match state_path with + | None -> + Context.set_repo_config ctx repo.url config; + Lwt.return ctx + | Some state_path -> + match Common.get_local_file state_path with + | Error e -> + log#error "failed to read %s: %s" state_path e; + Lwt.return ctx + | Ok file -> + let repo_state = State_j.repo_state_of_string file in + let%lwt () = State.set_repo_state ctx.state repo.url repo_state in + Context.set_repo_config ctx repo.url config; + Lwt.return ctx + in + Stdio.printf "===== file %s =====\n" path; + let headers = [ "x-github-event", kind ] in + match Common.get_local_file path with + | Error e -> Lwt.return @@ log#error "failed to read %s: %s" path e + | Ok event -> + let%lwt ctx = make_test_context event in + let%lwt _ctx = Action_local.process_github_notification ctx headers event in + Lwt.return_unit + with exn -> + log#error ~exn "failed to run github payload test from %s" path; + Caml.exit 1 let process_slack_event ~(secrets : Config_t.secrets) path = - let ctx = Context.make () in - ctx.secrets <- Some secrets; - State.set_bot_user_id ctx.state "bot_user"; - Stdio.printf "===== file %s =====\n" path; - match Common.get_local_file path with - | Error e -> Lwt.return @@ log#error "failed to read %s: %s" path e - | Ok body -> - match Slack_j.event_notification_of_string body with - | Url_verification _ -> Lwt.return () - | Event_callback notification -> - match notification.event with - | Link_shared event -> - let%lwt _ctx = Action_local.process_link_shared_event ctx event in - Lwt.return_unit + try%lwt + let ctx = Context.make () in + ctx.secrets <- Some secrets; + State.set_bot_user_id ctx.state "bot_user"; + Stdio.printf "===== file %s =====\n" path; + match Common.get_local_file path with + | Error e -> Lwt.return @@ log#error "failed to read %s: %s" path e + | Ok body -> + match Slack_j.event_notification_of_string body with + | Url_verification _ -> Lwt.return () + | Event_callback notification -> + match notification.event with + | Link_shared event -> + let%lwt _ctx = Action_local.process_link_shared_event ctx event in + Lwt.return_unit + with exn -> + log#error ~exn "failed to run slack unfurl test from %s" path; + Caml.exit 1 let () = let payloads = get_mock_payloads () in