Skip to content

Commit c0cddb0

Browse files
committed
state, slack: map comments + reviews to its issue
1 parent 3ff0d77 commit c0cddb0

10 files changed

+207
-167
lines changed

lib/action.ml

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
192192
partition_issue_comment cfg n |> List.map ~f:(generate_issue_comment_notification ctx n) |> Lwt.return
193193
| Commit_comment n ->
194194
let%lwt channels, api_commit = partition_commit_comment ctx n in
195-
let notifs = List.map ~f:(generate_commit_comment_notification ctx api_commit n) channels in
195+
let notifs = List.map ~f:(generate_commit_comment_notification api_commit n) channels in
196196
Lwt.return notifs
197197
| Status n ->
198198
let%lwt channels = partition_status ctx n in
@@ -203,10 +203,12 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
203203
let send_notifications (ctx : Context.t) (req : Github.t) notifications =
204204
let update_comment_mapping message =
205205
match req with
206-
| Github.PR_review_comment n -> State.add_comment_map ctx.state n.repository.url n.comment.id message
207-
| Issue_comment n -> State.add_comment_map ctx.state n.repository.url n.comment.id message
208-
| Commit_comment n -> State.add_comment_map ctx.state n.repository.url n.comment.id message
209-
| PR_review n -> State.add_review_map ctx.state n.repository.url n.review.id message
206+
| Github.PR_review_comment n ->
207+
State.add_comment_msg ctx.state n.repository.url ~issue_num:n.pull_request.number n.comment.id message
208+
| Issue_comment n ->
209+
State.add_comment_msg ctx.state n.repository.url ~issue_num:n.issue.number n.comment.id message
210+
| PR_review n ->
211+
State.add_review_msg ctx.state n.repository.url ~issue_num:n.pull_request.number n.review.id message
210212
| _ -> Lwt.return_unit
211213
in
212214
let notify = function

lib/slack.ml

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,10 @@ let generate_pr_review_notification (ctx : Context.t) notification channel =
112112
match action with
113113
| Submitted -> New_message { channel; text = Some summary; attachments; blocks = None }
114114
| Edited ->
115-
( match State.get_review_map ctx.state repository.url review.id with
115+
( match State.get_review_msg ctx.state repository.url ~issue_num:number review.id with
116116
| Some ({ channel; ts } : post_message_res) ->
117117
Update_message { channel; ts; text = Some summary; attachments; blocks = None }
118-
| None -> invalid_arg (sprintf "could not find comment %d in %s" review.id repository.url)
118+
| None -> invalid_arg (sprintf "could not find review %d in %s for PR #%n" review.id repository.url number)
119119
)
120120
| _ -> invalid_arg "impossible"
121121

@@ -155,10 +155,10 @@ let generate_pr_review_comment_notification (ctx : Context.t) notification chann
155155
match action with
156156
| Created -> New_message { channel; text = Some summary; attachments; blocks = None }
157157
| Edited ->
158-
( match State.get_comment_map ctx.state repository.url comment.id with
158+
( match State.get_comment_msg ctx.state repository.url ~issue_num:number comment.id with
159159
| Some ({ channel; ts } : post_message_res) ->
160160
Update_message { channel; ts; text = Some summary; attachments; blocks = None }
161-
| None -> invalid_arg (sprintf "could not find comment %d in %s" comment.id repository.url)
161+
| None -> invalid_arg (sprintf "could not find comment %d in %s for PR #%n" comment.id repository.url number)
162162
)
163163
| _ -> invalid_arg "impossible"
164164

@@ -229,10 +229,10 @@ let generate_issue_comment_notification (ctx : Context.t) notification channel =
229229
match action with
230230
| Created -> New_message { channel; text = Some summary; attachments; blocks = None }
231231
| Edited ->
232-
( match State.get_comment_map ctx.state repository.url comment.id with
232+
( match State.get_comment_msg ctx.state repository.url ~issue_num:number comment.id with
233233
| Some ({ channel; ts } : post_message_res) ->
234234
Update_message { channel; ts; text = Some summary; attachments; blocks = None }
235-
| None -> invalid_arg (sprintf "could not find comment %d in %s" comment.id repository.url)
235+
| None -> invalid_arg (sprintf "could not find comment %d in %s for issue %n" comment.id repository.url number)
236236
)
237237
| _ -> invalid_arg "impossible"
238238

@@ -371,9 +371,9 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_
371371
in
372372
New_message { channel; text = Some summary; attachments = Some [ attachment ]; blocks = None }
373373

374-
let generate_commit_comment_notification (ctx : Context.t) api_commit notification channel =
374+
let generate_commit_comment_notification api_commit notification channel =
375375
let { commit; _ } = api_commit in
376-
let { sender; comment; repository; action; _ } = notification in
376+
let { sender; comment; repository; _ } = notification in
377377
let commit_id =
378378
match comment.commit_id with
379379
| None -> invalid_arg "commit id not found"
@@ -398,15 +398,7 @@ let generate_commit_comment_notification (ctx : Context.t) api_commit notificati
398398
text = Some (mrkdwn_of_markdown comment.body);
399399
}
400400
in
401-
match action with
402-
| Created -> New_message { channel; text = Some summary; attachments = Some [ attachment ]; blocks = None }
403-
| Edited ->
404-
( match State.get_comment_map ctx.state repository.url comment.id with
405-
| Some ({ channel; ts } : post_message_res) ->
406-
Update_message { channel; ts; text = Some summary; attachments = Some [ attachment ]; blocks = None }
407-
| None -> invalid_arg (sprintf "could not find comment %d in %s" comment.id repository.url)
408-
)
409-
| _ -> invalid_arg "impossible"
401+
New_message { channel; text = Some summary; attachments = Some [ attachment ]; blocks = None }
410402

411403
let validate_signature ?(version = "v0") ?signing_key ~headers body =
412404
match signing_key with

lib/state.atd

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,17 @@ type pipeline_statuses = branch_statuses map_as_object
1212

1313
type post_message_res <ocaml from="Slack"> = abstract
1414
type post_msg_map = post_message_res map_as_object
15+
type issue_state = {
16+
comments <ocaml mutable>: post_msg_map;
17+
reviews <ocaml mutable>: post_msg_map;
18+
}
1519

16-
(* The runtime state of a given GitHub repository *)
20+
(* The runtime state of a given GitHub repository
21+
NB: issue number is the same as PR number for github APIs
22+
*)
1723
type repo_state = {
1824
pipeline_statuses <ocaml mutable>: pipeline_statuses;
19-
comments_map <ocaml mutable>: post_msg_map;
20-
reviews_map <ocaml mutable>: post_msg_map;
25+
issue_tbl <ocaml mutable>: issue_state table_as_object;
2126
}
2227

2328
(* The serializable runtime state of the bot *)

lib/state.ml

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,18 @@ type t = {
77
lock : Lwt_mutex.t; (** protect access to mutable string map `pipeline_statuses` *)
88
}
99

10-
let empty_repo_state () : State_t.repo_state =
11-
{ pipeline_statuses = StringMap.empty; comments_map = StringMap.empty; reviews_map = StringMap.empty }
10+
let empty_repo_state () : State_t.repo_state = { pipeline_statuses = StringMap.empty; issue_tbl = Stringtbl.empty () }
11+
let empty_issue_state () : State_t.issue_state = { comments = StringMap.empty; reviews = StringMap.empty }
1212

1313
let empty () : t =
1414
let state = State_t.{ repos = Stringtbl.empty (); bot_user_id = None } in
1515
{ state; lock = Lwt_mutex.create () }
1616

1717
let find_or_add_repo' state repo_url = Stringtbl.find_or_add state.State_t.repos repo_url ~default:empty_repo_state
1818

19+
let find_or_add_issue' (repo_state : State_t.repo_state) issue_num =
20+
Stringtbl.find_or_add repo_state.issue_tbl (Int.to_string issue_num) ~default:empty_issue_state
21+
1922
let set_repo_state { state; lock } repo_url repo_state =
2023
Lwt_mutex.with_lock lock @@ fun () ->
2124
Stringtbl.set state.repos ~key:repo_url ~data:repo_state;
@@ -35,25 +38,35 @@ let set_repo_pipeline_status { state; lock } repo_url ~pipeline ~(branches : Git
3538
repo_state.pipeline_statuses <- Map.update repo_state.pipeline_statuses pipeline ~f:set_branch_status;
3639
Lwt.return_unit
3740

38-
let add_comment_map { state; lock } repo_url comment_id post_message_res =
41+
let add_comment_msg { state; lock } repo_url ~issue_num comment_id post_message_res =
3942
Lwt_mutex.with_lock lock @@ fun () ->
4043
let repo_state = find_or_add_repo' state repo_url in
41-
repo_state.comments_map <- Map.set repo_state.comments_map ~key:(Int.to_string comment_id) ~data:post_message_res;
44+
let issue_tbl = find_or_add_issue' repo_state issue_num in
45+
issue_tbl.comments <- Map.set issue_tbl.comments ~key:(Int.to_string comment_id) ~data:post_message_res;
4246
Lwt.return_unit
4347

44-
let get_comment_map { state; _ } repo_url comment_id =
48+
let get_comment_msg { state; _ } repo_url ~issue_num comment_id =
4549
let repo_state = find_or_add_repo' state repo_url in
46-
Map.find repo_state.comments_map (Int.to_string comment_id)
50+
let issue_tbl = find_or_add_issue' repo_state issue_num in
51+
Map.find issue_tbl.comments (Int.to_string comment_id)
4752

48-
let add_review_map { state; lock } repo_url review_id post_message_res =
53+
let add_review_msg { state; lock } repo_url ~issue_num review_id post_message_res =
4954
Lwt_mutex.with_lock lock @@ fun () ->
5055
let repo_state = find_or_add_repo' state repo_url in
51-
repo_state.reviews_map <- Map.set repo_state.reviews_map ~key:(Int.to_string review_id) ~data:post_message_res;
56+
let issue_tbl = find_or_add_issue' repo_state issue_num in
57+
issue_tbl.reviews <- Map.set issue_tbl.reviews ~key:(Int.to_string review_id) ~data:post_message_res;
5258
Lwt.return_unit
5359

54-
let get_review_map { state; _ } repo_url review_id =
60+
let get_review_msg { state; _ } repo_url ~issue_num review_id =
5561
let repo_state = find_or_add_repo' state repo_url in
56-
Map.find repo_state.reviews_map (Int.to_string review_id)
62+
let issue_tbl = find_or_add_issue' repo_state issue_num in
63+
Map.find issue_tbl.reviews (Int.to_string review_id)
64+
65+
let close_issue { state; lock } repo_url issue_num =
66+
Lwt_mutex.with_lock lock @@ fun () ->
67+
let repo_state = find_or_add_repo' state repo_url in
68+
Stringtbl.remove repo_state.issue_tbl issue_num;
69+
Lwt.return_unit
5770

5871
let set_bot_user_id { state; _ } user_id = state.State_t.bot_user_id <- Some user_id
5972
let get_bot_user_id { state; _ } = state.State_t.bot_user_id
Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,27 @@
11

22
{
3-
"pipeline_statuses": {
4-
"default": {
5-
"master": "failure"
3+
"pipeline_statuses": {
4+
"default": {
5+
"master": "failure"
6+
},
7+
"buildkite/pipeline2": {
8+
"master": "success"
9+
}
610
},
7-
"buildkite/pipeline2": {
8-
"master": "success"
11+
"issue_tbl" : {
12+
"4" : {
13+
"comments": {
14+
"0": {
15+
"channel": "some channel",
16+
"ts": "some ts"
17+
}
18+
},
19+
"reviews": {
20+
"0": {
21+
"channel": "some channel",
22+
"ts": "some ts"
23+
}
24+
}
25+
}
926
}
10-
},
11-
"comments_map": {
12-
"0": {
13-
"channel": "some channel",
14-
"ts": "some ts"
15-
}
16-
},
17-
"reviews_map": {
18-
"0": {
19-
"channel": "some channel",
20-
"ts": "some ts"
21-
}
22-
}
2327
}
Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,27 @@
11

22
{
3-
"pipeline_statuses": {
4-
"default": {
5-
"master": "failure"
3+
"pipeline_statuses": {
4+
"default": {
5+
"master": "failure"
6+
},
7+
"buildkite/pipeline2": {
8+
"master": "success"
9+
}
610
},
7-
"buildkite/pipeline2": {
8-
"master": "success"
11+
"issue_tbl" : {
12+
"4": {
13+
"comments": {
14+
"0": {
15+
"channel": "some channel",
16+
"ts": "some ts"
17+
}
18+
},
19+
"reviews": {
20+
"0": {
21+
"channel": "some channel",
22+
"ts": "some ts"
23+
}
24+
}
25+
}
926
}
10-
},
11-
"comments_map": {
12-
"0": {
13-
"channel": "some channel",
14-
"ts": "some ts"
15-
}
16-
},
17-
"reviews_map": {
18-
"0": {
19-
"channel": "some channel",
20-
"ts": "some ts"
21-
}
22-
}
2327
}
Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,27 @@
11

22
{
3-
"pipeline_statuses": {
4-
"default": {
5-
"master": "failure"
3+
"pipeline_statuses": {
4+
"default": {
5+
"master": "failure"
6+
},
7+
"buildkite/pipeline2": {
8+
"master": "success"
9+
}
610
},
7-
"buildkite/pipeline2": {
8-
"master": "success"
11+
"issue_tbl" : {
12+
"4": {
13+
"comments": {
14+
"0": {
15+
"channel": "some channel",
16+
"ts": "some ts"
17+
}
18+
},
19+
"reviews": {
20+
"0": {
21+
"channel": "some channel",
22+
"ts": "some ts"
23+
}
24+
}
25+
}
926
}
10-
},
11-
"comments_map": {
12-
"0": {
13-
"channel": "some channel",
14-
"ts": "some ts"
15-
}
16-
},
17-
"reviews_map": {
18-
"0": {
19-
"channel": "some channel",
20-
"ts": "some ts"
21-
}
22-
}
2327
}
Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,27 @@
11

22
{
3-
"pipeline_statuses": {
4-
"default": {
5-
"master": "failure"
3+
"pipeline_statuses": {
4+
"default": {
5+
"master": "failure"
6+
},
7+
"buildkite/pipeline2": {
8+
"master": "success"
9+
}
610
},
7-
"buildkite/pipeline2": {
8-
"master": "success"
11+
"issue_tbl" : {
12+
"4": {
13+
"comments": {
14+
"0": {
15+
"channel": "some channel",
16+
"ts": "some ts"
17+
}
18+
},
19+
"reviews": {
20+
"0": {
21+
"channel": "some channel",
22+
"ts": "some ts"
23+
}
24+
}
25+
}
926
}
10-
},
11-
"comments_map": {
12-
"0": {
13-
"channel": "some channel",
14-
"ts": "some ts"
15-
}
16-
},
17-
"reviews_map": {
18-
"0": {
19-
"channel": "some channel",
20-
"ts": "some ts"
21-
}
22-
}
2327
}

0 commit comments

Comments
 (0)