常に「レビューを依頼されました」になってる
適切な文言かタイミングで文言が変わるようにする
アサインについても同様(常に「アサインされました」になる)。
方針
原因
GitHub の /notifications API の reason は「そのスレッドを購読している理由」であり、イベント種別ではない。一度レビュー依頼/アサインされた PR・Issue は、以降のコメントや更新もすべて同じ reason で届くため、REASON_MESSAGES(src/github/models.cr)の固定対応だと常に「レビューを依頼されました」「アサインされました」になる。
対応方針
追加の API コールなしで、既存レスポンスのフィールドから「依頼・アサイン直後」か「その後」かを推定して文言を切り替える。
判定には subject.latest_comment_url を使うが、空かどうかではなく subject.url と一致するかで見る。latest_comment_url はコメントがまだ無いスレッドでは空にならず subject.url と同じ値が入るため、空判定だとコメント無しの通知を誤判定してしまう。
latest_comment_url が空、または subject.url と同じ → コメントが無い=直後とみなし、現行どおりの文言
latest_comment_url が subject.url と異なる → スレッドにコメントがある=初回ではないので差し替える
| reason |
スレッドの状態 |
文言 |
review_requested |
コメント無し |
レビューを依頼されました(従来どおり) |
review_requested |
コメントあり |
レビュー依頼中の PR に動きがありました |
assign |
コメント無し |
アサインされました(従来どおり) |
assign |
コメントあり |
担当している PR/Issue に動きがありました |
assign は Issue にも付くため、文言は PR に限定しない。
文言を「動きがありました」に留める理由
latest_comment_url は通知を発生させたイベントではなく、スレッドの現在の最新コメントを指す。そのため、コメント済みスレッドに push や状態変更が来た通知でも判定は真になる。ここで「コメントがつきました」と書くと過去のコメントを新着と誤認させるため、何が起きたかを限定しない表現にする。
イベント種別は通知 payload に無く、追加 API なしでは判別できない。この判定はあくまで「初回ではない」ことの目安として使う。
実装箇所
src/github/models.cr: Subject#commented? を追加し、Notification#reason_message で FOLLOWUP_MESSAGES による差し替えを行う。テーブル方式にして他の reason にも展開できるようにする
spec/github/models_spec.cr / spec/github/usecase_spec.cr: commented? の各パターンと文言の切り替えをテスト
対象外にした reason
同じ構造の問題を持つ reason は他に author と invitation があるが、author は元の文言(自分の PR/Issue に動きがありました)が後続通知でも実態と合っており、invitation は後続通知がほぼ発生しないため据え置く。必要になれば FOLLOWUP_MESSAGES への追記だけで対応できる。
スコープ外
「実際にまだレビュー待ちかどうか」(レビュー済みか、re-request されたか)の正確な判定には PR の requested_reviewers の取得が必要で、毎分実行での API コスト増に見合わないため今回はやらない。必要になったら別 issue で検討する。
対応 PR
#106(CI green)
常に「レビューを依頼されました」になってる
適切な文言かタイミングで文言が変わるようにする
アサインについても同様(常に「アサインされました」になる)。
方針
原因
GitHub の
/notificationsAPI のreasonは「そのスレッドを購読している理由」であり、イベント種別ではない。一度レビュー依頼/アサインされた PR・Issue は、以降のコメントや更新もすべて同じ reason で届くため、REASON_MESSAGES(src/github/models.cr)の固定対応だと常に「レビューを依頼されました」「アサインされました」になる。対応方針
追加の API コールなしで、既存レスポンスのフィールドから「依頼・アサイン直後」か「その後」かを推定して文言を切り替える。
判定には
subject.latest_comment_urlを使うが、空かどうかではなくsubject.urlと一致するかで見る。latest_comment_urlはコメントがまだ無いスレッドでは空にならずsubject.urlと同じ値が入るため、空判定だとコメント無しの通知を誤判定してしまう。latest_comment_urlが空、またはsubject.urlと同じ → コメントが無い=直後とみなし、現行どおりの文言latest_comment_urlがsubject.urlと異なる → スレッドにコメントがある=初回ではないので差し替えるreview_requestedreview_requestedassignassignassignは Issue にも付くため、文言は PR に限定しない。文言を「動きがありました」に留める理由
latest_comment_urlは通知を発生させたイベントではなく、スレッドの現在の最新コメントを指す。そのため、コメント済みスレッドに push や状態変更が来た通知でも判定は真になる。ここで「コメントがつきました」と書くと過去のコメントを新着と誤認させるため、何が起きたかを限定しない表現にする。イベント種別は通知 payload に無く、追加 API なしでは判別できない。この判定はあくまで「初回ではない」ことの目安として使う。
実装箇所
src/github/models.cr:Subject#commented?を追加し、Notification#reason_messageでFOLLOWUP_MESSAGESによる差し替えを行う。テーブル方式にして他の reason にも展開できるようにするspec/github/models_spec.cr/spec/github/usecase_spec.cr:commented?の各パターンと文言の切り替えをテスト対象外にした reason
同じ構造の問題を持つ reason は他に
authorとinvitationがあるが、authorは元の文言(自分の PR/Issue に動きがありました)が後続通知でも実態と合っており、invitationは後続通知がほぼ発生しないため据え置く。必要になればFOLLOWUP_MESSAGESへの追記だけで対応できる。スコープ外
「実際にまだレビュー待ちかどうか」(レビュー済みか、re-request されたか)の正確な判定には PR の
requested_reviewersの取得が必要で、毎分実行での API コスト増に見合わないため今回はやらない。必要になったら別 issue で検討する。対応 PR
#106(CI green)