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.

Sign up for GitHub

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

Jump to bottom

Use plaintext in fallback and escape user text fields in Slack messages #105

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Use plaintext in fallback and escape user text fields in Slack messages #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e222cd
use plaintext for all fallback messages
yasunariw Jan 5, 2021
46b0420
escape all user text and provide pretty-printing utility functions
yasunariw Jan 5, 2021
ee52fb2
tests: promote with simple char escape check
yasunariw Jan 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 68 additions & 43 deletions lib/slack.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ let show_labels = function

let pluralize name num suffix = if num = 1 then sprintf "%s" name else String.concat [ name; suffix ]

let escape = Mrkdwn.escape_mrkdwn

let pp_link url text = sprintf "<%s|%s>" url (escape text)

let pp_repo (repo : repository) = sprintf "[%s]" repo.full_name

let pp_repo_mrkdwn (repo : repository) = pp_link repo.url @@ pp_repo repo

let generate_pull_request_notification notification channel =
let { action; number; sender; pull_request; repository } = notification in
let ({ body; title; html_url; labels; _ } : pull_request) = pull_request in
Expand All @@ -50,8 +58,11 @@ let generate_pull_request_notification notification channel =
in
let summary =
Some
(sprintf "<%s|[%s]> Pull request #%d <%s|%s> %s by *%s*" repository.url repository.full_name number html_url title
action sender.login)
(sprintf "%s Pull request #%d %s %s by *%s*" (pp_repo_mrkdwn repository) number (pp_link html_url title) action
sender.login)
in
let fallback =
Some (sprintf "%s Pull request #%d %s %s by %s" (pp_repo repository) number title action sender.login)
in
{
channel;
Expand All @@ -62,7 +73,7 @@ let generate_pull_request_notification notification channel =
{
empty_attachments with
mrkdwn_in = Some [ "text" ];
fallback = summary;
fallback;
color = Some "#ccc";
pretext = summary;
text = mrkdwn_of_markdown_opt body;
Expand Down Expand Up @@ -90,9 +101,10 @@ let generate_pr_review_notification notification channel =
in
let summary =
Some
(sprintf "<%s|[%s]> *%s* <%s|%s> #%d <%s|%s>" repository.url repository.full_name sender.login review.html_url
action_str number html_url title)
(sprintf "%s *%s* %s #%d %s" (pp_repo_mrkdwn repository) sender.login (pp_link review.html_url action_str) number
(pp_link html_url title))
in
let fallback = Some (sprintf "%s %s %s #%d %s" (pp_repo repository) sender.login action_str number title) in
{
channel;
text = None;
Expand All @@ -102,7 +114,7 @@ let generate_pr_review_notification notification channel =
{
empty_attachments with
mrkdwn_in = Some [ "text" ];
fallback = summary;
fallback;
color = Some "#ccc";
pretext = summary;
text = mrkdwn_of_markdown_opt review.body;
Expand All @@ -124,13 +136,14 @@ let generate_pr_review_comment_notification notification channel =
in
let summary =
Some
(sprintf "<%s|[%s]> *%s* %s on #%d <%s|%s>" repository.url repository.full_name sender.login action_str number
html_url title)
(sprintf "%s *%s* %s on #%d %s" (pp_repo_mrkdwn repository) sender.login action_str number
(pp_link html_url title))
in
let fallback = Some (sprintf "%s %s %s on #%d %s" (pp_repo repository) sender.login action_str number title) in
let file =
match comment.path with
| None -> None
| Some a -> Some (sprintf "New comment by %s in <%s|%s>" sender.login comment.html_url a)
| Some a -> Some (sprintf "New comment by %s in %s" sender.login (pp_link comment.html_url a))
in
{
channel;
Expand All @@ -141,7 +154,7 @@ let generate_pr_review_comment_notification notification channel =
{
empty_attachments with
mrkdwn_in = Some [ "text" ];
fallback = summary;
fallback;
color = Some "#ccc";
pretext = summary;
footer = file;
Expand All @@ -167,9 +180,10 @@ let generate_issue_notification notification channel =
in
let summary =
Some
(sprintf "<%s|[%s]> Issue #%d <%s|%s> %s by *%s*" repository.url repository.full_name number html_url title action
(sprintf "%s Issue #%d %s %s by *%s*" (pp_repo_mrkdwn repository) number (pp_link html_url title) action
sender.login)
in
let fallback = Some (sprintf "%s Issue #%d %s %s by %s" (pp_repo repository) number title action sender.login) in
{
channel;
text = None;
Expand All @@ -179,7 +193,7 @@ let generate_issue_notification notification channel =
{
empty_attachments with
mrkdwn_in = Some [ "text" ];
fallback = summary;
fallback;
color = Some "#ccc";
pretext = summary;
text = mrkdwn_of_markdown_opt body;
Expand All @@ -202,9 +216,10 @@ let generate_issue_comment_notification notification channel =
in
let summary =
Some
(sprintf "<%s|[%s]> *%s* <%s|%s> on #%d <%s|%s>" repository.url repository.full_name sender.login comment.html_url
action_str number issue.html_url title)
(sprintf "%s *%s* %s on #%d %s" (pp_repo_mrkdwn repository) sender.login (pp_link comment.html_url action_str)
number (pp_link issue.html_url title))
in
let fallback = Some (sprintf "%s %s %s on #%d %s" (pp_repo repository) sender.login action_str number title) in
{
channel;
text = None;
Expand All @@ -214,7 +229,7 @@ let generate_issue_comment_notification notification channel =
{
empty_attachments with
mrkdwn_in = Some [ "text" ];
fallback = summary;
fallback;
color = Some "#ccc";
pretext = summary;
text = Some (mrkdwn_of_markdown comment.body);
Expand All @@ -229,35 +244,36 @@ let generate_push_notification notification channel =
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
let repo_branch = sprintf "[%s:%s]" repository.name commits_branch in
let num_commits = sprintf "%d %s" (List.length commits) (pluralize "commit" (List.length commits) "s") in
let action = if forced then "force-pushed" else "pushed" in
let dest = if created then "to new branch " else "" in
let title =
if deleted then
sprintf "<%s|[%s]> %s deleted branch <%s|%s>" tree_url repository.name sender.login compare commits_branch
sprintf "%s %s deleted branch %s" (pp_link tree_url repository.name) sender.login (pp_link compare commits_branch)
else
sprintf "<%s|[%s:%s]> <%s|%i commit%s> %spushed %sby %s" tree_url repository.name commits_branch compare
(List.length commits)
( match commits with
| [ _ ] -> ""
| _ -> "s"
)
(if forced then "force-" else "")
(if created then "to new branch " else "")
sender.login
sprintf "%s %s %s %sby %s" (pp_link tree_url repo_branch) (pp_link compare num_commits) action dest sender.login
in
let fallback =
if deleted then sprintf "%s %s deleted branch %s" (pp_repo repository) sender.login commits_branch
else sprintf "%s %s %s %sby %s" repo_branch num_commits action dest sender.login
in
let commits =
List.map commits ~f:(fun { url; id; message; author; _ } ->
let title = first_line message in
sprintf "`<%s|%s>` %s - %s" url (git_short_sha_hash id) title author.name)
let title = escape @@ first_line message in
sprintf "`%s` %s - %s" (pp_link url (git_short_sha_hash id)) title author.name)
in
{
channel;
text = Some title;
text = None;
attachments =
Some
[
{
empty_attachments with
mrkdwn_in = Some [ "fields" ];
fallback = Some "Commit pushed notification";
pretext = Some title;
fallback = Some fallback;
color = Some "#ccc";
fields = Some [ { value = String.concat ~sep:"\n" commits; title = None; short = false } ];
};
Expand Down Expand Up @@ -290,7 +306,12 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_
| Some s -> Some (sprintf "*Description*: %s." s)
in
let commit_info =
[ sprintf "*Commit*: `<%s|%s>` %s - %s" html_url (git_short_sha_hash sha) (first_line message) author.login ]
[
sprintf "*Commit*: `%s` %s - %s"
(pp_link html_url (git_short_sha_hash sha))
(escape @@ first_line message)
author.login;
]
in
let branches_info =
match List.map ~f:(fun { name } -> name) notification.branches with
Expand All @@ -306,21 +327,19 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_
[ sprintf "*%s*: %s" (pluralize "Branch" (List.length branches) "es") (String.concat ~sep:", " branches) ]
in
let summary =
match target_url with
| None ->
Some (sprintf "<%s|[%s]> CI Build Status notification: %s" repository.url repository.full_name state_info)
(* in case the CI run is not using buildkite *)
| Some t ->
Some
(sprintf "<%s|[%s]> CI Build Status notification for <%s|%s>: %s" repository.url repository.full_name t context
state_info)
let target = Option.value_map target_url ~default:"" ~f:(fun url -> sprintf " for %s" (pp_link url context)) in
Some (sprintf "%s CI Build Status notification%s: %s" (pp_repo_mrkdwn repository) target state_info)
in
let fallback =
let target = Option.value_map target_url ~default:"" ~f:(fun _ -> sprintf " for %s" context) in
Some (sprintf "%s CI Build Status notification%s: %s" (pp_repo repository) target 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" ];
fallback = summary;
fallback;
pretext = summary;
color = Some color_info;
text = description_info;
Expand All @@ -339,19 +358,25 @@ let generate_commit_comment_notification api_commit notification channel =
in
let summary =
Some
(sprintf "<%s|[%s]> *%s* commented on `<%s|%s>` %s" repository.url repository.full_name sender.login
comment.html_url (git_short_sha_hash commit_id) (first_line commit.message))
(sprintf "%s *%s* commented on `%s` %s" (pp_repo_mrkdwn repository) sender.login
(pp_link comment.html_url (git_short_sha_hash commit_id))
(escape @@ first_line commit.message))
in
let fallback =
Some
(sprintf "%s %s commented on `%s` %s" (pp_repo repository) sender.login (git_short_sha_hash commit_id)
(first_line commit.message))
in
let path =
match comment.path with
| None -> None
| Some p -> Some (sprintf "New comment by %s in <%s|%s>" sender.login comment.html_url p)
| Some p -> Some (sprintf "New comment by %s in %s" sender.login (pp_link comment.html_url p))
in
let attachment =
{
empty_attachments with
mrkdwn_in = Some [ "pretext"; "text" ];
fallback = summary;
fallback;
color = Some "#ccc";
pretext = summary;
footer = path;
Expand Down
2 changes: 1 addition & 1 deletion mock_payloads/pull_request_review_comment.created.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"type": "User",
"site_admin": false
},
"body": "PR review comment example",
"body": "PR review comment example with <escaped & characters>",
"created_at": "2020-05-25T07:37:52Z",
"updated_at": "2020-05-25T07:38:26Z",
"html_url": "https://github.com/xinyuluo/monorepo/pull/4#discussion_r0",
Expand Down
Loading