Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
82 changes: 82 additions & 0 deletions spec/github/models_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,48 @@ describe Github::Notification do
)
notification.reason_message.should eq "コメントがつきました"
end

# latest_comment_url が subject.url に戻る通知でも、取得済みの subject 本体の
# コメント数で初回でないと判断できる(issue #116)。
it "switches to a follow-up message when the fetched pull request has conversation comments" do
message = pull_request_without_comment_signal.reason_message(subject_detail(comments: 2, review_comments: 0))
message.should eq "担当している PR/Issue に動きがありました"
end

it "switches to a follow-up message when the fetched pull request only has review comments" do
message = pull_request_without_comment_signal.reason_message(subject_detail(comments: 0, review_comments: 3))
message.should eq "担当している PR/Issue に動きがありました"
end

it "switches to a follow-up message when the fetched issue has comments" do
notification = notification_from(
"assign",
url: "https://api.github.com/repos/o/r/issues/1",
latest_comment_url: "https://api.github.com/repos/o/r/issues/1",
)
notification.reason_message(subject_detail(comments: 1)).should eq "担当している PR/Issue に動きがありました"
end

it "keeps the assign message when the fetched pull request has no comment at all" do
message = pull_request_without_comment_signal.reason_message(subject_detail(comments: 0, review_comments: 0))
message.should eq "アサインされました"
end

it "keeps the assign message when the fetched payload carries no comment count" do
# 本文取得に失敗した場合など、判断材料が無いときは初回向け文言のままにする
pull_request_without_comment_signal.reason_message(subject_detail).should eq "アサインされました"
end
end

describe "#pretext" do
it "prefixes the subject type before the reason message" do
notification_from("mention").pretext.should eq "[Issue] メンションされました"
end

it "passes the fetched subject detail through to the reason message" do
pretext = pull_request_without_comment_signal.pretext(subject_detail(comments: 1))
pretext.should eq "[PullRequest] 担当している PR/Issue に動きがありました"
end
end

describe "#display_title" do
Expand Down Expand Up @@ -255,6 +291,52 @@ private def notification_from(reason : String, type = "Issue", url = "", latest_
}.to_json)
end

# latest_comment_url が subject.url に戻っている(=コメントの有無を判定できない)
# アサイン済み PR の通知(issue #116)。
private def pull_request_without_comment_signal
notification_from(
"assign",
type: "PullRequest",
url: "https://api.github.com/repos/o/r/pulls/1",
latest_comment_url: "https://api.github.com/repos/o/r/pulls/1",
)
end

# 本文取得で返る subject 本体(PR / Issue)のレスポンス。件数を省くと
# コメントオブジェクト(件数フィールドを持たない)と同じ形になる。
private def subject_detail(comments : Int32? = nil, review_comments : Int32? = nil)
Github::Comment.from_json({
body: "body",
user: {login: "octocat"},
comments: comments,
review_comments: review_comments,
}.to_json)
end

describe Github::Comment do
describe "#commented?" do
it "is true when the pull request has conversation comments" do
subject_detail(comments: 2, review_comments: 0).commented?.should be_true
end

it "is true when the pull request only has review comments" do
subject_detail(comments: 0, review_comments: 3).commented?.should be_true
end

it "is false when the pull request has no comment at all" do
subject_detail(comments: 0, review_comments: 0).commented?.should be_false
end

it "is false when the payload has no comment count (a comment object)" do
subject_detail.commented?.should be_false
end

it "is false for a locally built comment (no comment url / fetch failure)" do
Github::Comment.new(nil).commented?.should be_false
end
end
end

private def notification_with(url = "", repo_html_url : String? = nil)
Github::Notification.from_json({
reason: "subscribed",
Expand Down
24 changes: 24 additions & 0 deletions spec/github/usecase_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ private def comment(body : String? = "body", html_url : String? = "https://examp
Github::Comment.from_json({body: body, html_url: html_url, user: {login: "octocat"}}.to_json)
end

# コメントが無いスレッドで本文取得先になる subject 本体(PR / Issue)のレスポンス。
# 件数を省くとコメントオブジェクトと同じ形になる(issue #116)。
private def subject_detail(comments : Int32? = nil, review_comments : Int32? = nil)
Github::Comment.from_json({
body: "body",
html_url: "https://example.com/c",
user: {login: "octocat"},
comments: comments,
review_comments: review_comments,
}.to_json)
end

private def build(notify, comment)
Github::Usecase.new(StubRepo.new(comment)).build_message(notify)
end
Expand All @@ -57,6 +69,18 @@ describe Github::Usecase do
build(notify, comment).pretext.should eq "[Issue] レビュー依頼中の PR に動きがありました"
end

# latest_comment_url からはコメントの有無が分からない通知でも、本文取得で
# 得た subject 本体のコメント数で文言を切り替える(issue #116)。
it "reflects the follow-up wording when the fetched pull request has comments" do
message = build(pull_request(reason: "assign"), subject_detail(comments: 1, review_comments: 0))
message.pretext.should eq "[PullRequest] 担当している PR/Issue に動きがありました"
end

it "keeps the initial wording when the fetched pull request has no comment" do
message = build(pull_request(reason: "assign"), subject_detail(comments: 0, review_comments: 0))
message.pretext.should eq "[PullRequest] アサインされました"
end

it "formats the title as owner/repo#number title" do
build(notification, comment).title.should eq "octocat/Hello-World#42 Spurious failure"
end
Expand Down
68 changes: 58 additions & 10 deletions src/github/models.cr
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ module Github
# 何が起きたか(コメントか push か状態変更か)は通知 payload から判別できない
# ため、文言はコメントに限定せず「動きがありました」に留める。
# ここに reason を足せば他の reason にも同じ切り替えを適用できる。
#
# 切り替えの判定は Subject#commented? だけでは足りず、本文取得で得た subject
# 本体のコメント数も併用する(issue #116。詳細は followup? のコメント)。
FOLLOWUP_MESSAGES = {
"review_requested" => "レビュー依頼中の PR に動きがありました",
"assign" => "担当している PR/Issue に動きがありました",
Expand All @@ -91,15 +94,41 @@ module Github
reason.in?(MENTION_REASONS)
end

def reason_message : String
if subject.commented?
# reason に対応する文言。detail には本文取得で得た subject 本体(PR / Issue)を
# 渡す。コメントの有無の判定に使うだけなので、渡さなければ従来どおり
# Subject#commented? のみで判定する。
def reason_message(detail : Comment? = nil) : String
if followup?(detail)
followup = FOLLOWUP_MESSAGES[reason]?
return followup if followup
end

REASON_MESSAGES[reason]? || GENERIC_MESSAGE
end

# 「初回ではない=その後の動き」とみなせるか。
#
# Subject#commented?(latest_comment_url が subject.url と異なる)だけでは
# 取りこぼす。latest_comment_url は通知を発生させたイベント側を反映することが
# あり、コメント済みのスレッドでも push・レビュー・アサイン変更が起点の通知では
# subject.url に戻る。またレビューコメントは latest_comment_url に現れない
# ことがあるため、レビュー上でだけ議論されている PR は常に初回扱いになる。
# 結果、コメントの付いた PR でも「アサインされました」のままになる(issue #116)。
#
# そこで本文表示のためにどのみち取得している subject 本体のレスポンスを使う。
# Subject#commented? が false のとき comment_url は PR / Issue 自体を指し、
# そのレスポンスにはコメント数(PR は会話とレビューの 2 種)が含まれるので、
# 追加の API 呼び出し無しでスレッドにコメントがあるかを判定できる。
#
# 逆に Subject#commented? が true のときは取得先が実コメントでコメント数を
# 持たないが、その場合は先に true が確定するので影響しない。
private def followup?(detail : Comment?) : Bool
return true if subject.commented?
return false unless detail

detail.commented?
end

# CI・自動チェックの状態でメンションを抑止する対象か(issue #105)。
# レビューできる状態になっていない PR で `@channel` / `@everyone` を撃たない
# ことが目的なので、PR の通知はすべて対象にする。
Expand All @@ -118,8 +147,9 @@ module Github
end

# 通知の pretext(botのセリフ)。`[<type>] <reason 文言>` 形式。
def pretext : String
"[#{subject.type}] #{reason_message}"
# detail は reason_message にそのまま渡す(issue #116)。
def pretext(detail : Comment? = nil) : String
"[#{subject.type}] #{reason_message(detail)}"
end

# 一目で対象が分かるよう `owner/repo#番号 タイトル` 形式にする。
Expand Down Expand Up @@ -211,12 +241,15 @@ module Github
# 値が入る。よって url と異なる値のときだけコメントありと判断できる。
#
# あくまで「コメントが存在するか」であって「今回の通知の起点がコメントか」では
# ない点に注意。latest_comment_url は通知を発生させたイベントではなくスレッドの
# 現在の最新コメントを指すため、コメント済みスレッドに push や状態変更が来た
# 通知でも真になる。通知 payload にイベント種別が無く、追加の API 呼び出し
# 無しでは区別できないので、これを「初回ではない=その後の動き」の目安として
# 使い、文言側はコメントに限定しない表現にしている(issue #104 / PR #106
# レビュー指摘)。
# ない点に注意。通知 payload にイベント種別が無く、追加の API 呼び出し無しでは
# 区別できないので、これを「初回ではない=その後の動き」の目安として使い、
# 文言側はコメントに限定しない表現にしている(issue #104 / PR #106 レビュー指摘)。
#
# ただし真になるのは片道で、これが偽でもコメントが無いとは限らない。
# latest_comment_url は通知を発生させたイベント側を反映することがあり、
# コメント済みでも push などが起点の通知では subject.url に戻る。よって
# 「コメントが無い」側の確定には使えず、Notification#followup? では subject
# 本体のコメント数と併用する(issue #116)。
def commented? : Bool
return false unless comment = latest_comment_url.presence
comment != url
Expand All @@ -240,16 +273,31 @@ module Github
getter owner : User
end

# 通知本文の取得結果。コメントが無いスレッドでは subject 本体(PR / Issue)を
# 取得するため、コメントと subject 本体の両方をこの 1 クラスで受ける。
class Comment
include JSON::Serializable

getter user : User
getter html_url : String?
getter body : String?
# スレッドのコメント数。subject 本体を取得したときだけ入り、コメント
# オブジェクトのレスポンスには無いため nilable(issue #116)。
# PR は会話タブ(comments)とレビュー(review_comments)で別カウントになる。
getter comments : Int32?
getter review_comments : Int32?

def initialize(@body)
@user = User.new
end

# スレッドにコメントが 1 件以上付いているか(issue #116)。
# 件数が取れない場合(コメントオブジェクト・本文取得失敗・本文なし通知)は
# 判断材料が無いので false を返し、呼び出し側で初回向け文言に倒す。
def commented? : Bool
total = (comments || 0) + (review_comments || 0)
total.positive?
end
end

class User
Expand Down
4 changes: 3 additions & 1 deletion src/github/usecase.cr
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ module Github
author_name: comment.user.login,
author_icon: comment.user.avatar_url,
author_link: comment.user.html_url,
pretext: notify.pretext,
# コメントが無いスレッドでは comment は subject 本体(PR / Issue)になる。
# その中のコメント数を文言の切り替え判定に使う(issue #116)。
pretext: notify.pretext(comment),
color: notify.subject.color,
title: notify.display_title,
title_link: notify.link(comment),
Expand Down
Loading