コード内のメンションは通知対象外にするように実装

[reckyy]

Issue

• https://github.com/fjordllc/bootcamp/issues/1938

概要

Issueの概要の通りです。

変更確認方法

1. bugfix/exclude-@mentor-in-the-code-from-notification をローカルに取り込む
2. foreman start -f Procfile.dev
3. ID kimuraでログイン
4. 日報の本文内容を記述し、投稿。（例は一斉通知と、個別通知を一括確認する例になっています。）
5. ID komagataでログインし、通知が来ていないことを確認。

日報記述例

``` @mentor ```

@mentor

@komagata 

@komagata

Screenshot

手順実行後の通知です。

変更前

変更後

補足

コードブロック、インラインコード内でメンションを行っても通知が送られるのは日報だけでした。 他に考えられるシチュエーションとしては以下で、全てチェックし通知が来ないことを確認しました。 他に可能性がある事項があれば、教えていただきたいです！

• [x] 質問
• [x] 質問への回答
• [x] 提出物へのコメント
• [x] 提出物
• [x] Docs投稿
• [x] Docsへのコメント
• [x] イベント作成
• [x] イベントへのコメント
• [x] お知らせへのコメント
• [x] ポートフォリオ投稿

[reckyy]

@mousu-a お疲れ様です！ こちらのPRのレビューをお願いしたく、ご連絡いたしました。 お手隙の際にご対応いただけると嬉しいです。全く急ぎではないです。 もし、ご都合悪ければ仰ってください！ よろしくお願いいたします。

[mousu-a]

@reckyy

レビュー承知いたしました〜！ 5日以内にはお返しできると思います。(遅くなったら申し訳ないです🙇‍♂️) 今しばらくお待ちください〜🙏

ところで今日はQ&Aお答えいただきありがとうございました😄

[mousu-a]

@reckyy

まだメンションいただいていないのでフライングかもしれませんが🙏 お早い修正ありがとうございます！！

すいません。こちら影響する箇所が複数あったため、引数の数を元に戻しています。 使用回数は一回に修正したままになっているはずです！

extract_login_names_from_mentionsの引数を1つに出来たのはとても良かったと思いますー！よく考えたらfind_users_from_mentionsの中でやる必要はありませんでしたね。気づきませんでした😅

---

ところでこれは修正要望とかそういうことじゃなくて、ご意見を伺いたいだけです！ すみません！言葉足らずでした🙏 修正いただきましたこちらのコードについてです！

mentionable_without_code = mentionable.gsub(/```.*?```|`.*?`/m, '')

今回の修正のようにワンライナーで書くことは僕も考えたのですが、それだと「この正規表現は何をしているのか」内容を確かめる必要性が多少なりとも発生しコンテキストが上がる気がします。

僕的には「何をしているのか」をメソッドとして切り出してあげると、メソッド名が短いコメントとして機能し、ぱっと見て「あ〜そういうことしてるのね」という感じで正規表現の中身を見なくても良くなるかな？と思いメソッドに切り出した次第です。

この辺りreckyyさんのご意見を伺いたいですー！🙇‍♂️

[reckyy]

@mousu-a お疲れ様です！こちらこそ素早いご確認ありがとうございます〜！

まだメンションいただいていないのでフライングかもしれませんが🙏

CIが通ってから、メンションしようと思ってました！なので、修正内容の最終確認はCIが通るのをお待ちください🙏

先にいただいたご意見についてですが、

僕的には「何をしているのか」をメソッドとして切り出してあげると、メソッド名が短いコメントとして機能し、ぱっと見て「あ〜そういうことしてるのね」という感じで正規表現の中身を見なくても良くなるかな？と思いメソッドに切り出した次第です。

こちらは具体的なイメージがあったりしますでしょうか？👀 というのも、「この正規表現は何をしているのか」をうまく表現できるメソッド名（変数名とは異なる）を自分が考えつかないためです。。 以下のようなイメージでしょうか？

mentionable_without_code = remove_codes_from_report

def remove_codes_from_report
  mentionable.gsub(/```.*?```|`.*?`/m, '')
end

[mousu-a]

@reckyy

お疲れ様です！

CIが通ってから、メンションしようと思ってました！なので、修正内容の最終確認はCIが通るのをお待ちください🙏

やっぱり早かったですね😅了解です〜🙏

---

こちらは具体的なイメージがあったりしますでしょうか？👀

シンプルに「不要なメンションを取り除く」とかどうでしょうか。👀 あとは「正しいメンションを抽出する」とか？ except_unnecessary_mentions,correct_mentionsとか... (細かくて申し訳ないんですが、removeは破壊的なイメージがあります。exceptとかどうでしょう？)

extract_login_names_from_mentionsにならってextract_correct_mentionsとか？（英語力が皆無なので微妙だったらすみません😭）

自信がない、没にした案

個人的には「filter」という表現も気に入っていますがこれだと抽象的すぎますかね〜...🤔

あるいはそれこそもっと具体的に「コードブロック内のメンションを取り除く」とかでしょうか。（これは具体的すぎて微妙かも）

メンバー変数的なメソッド名も考えましたが、その場合おかしくなってしまう気がしたのでやめました。例：mentionable_without_code = 正しいメンション

---

つらつらとあげましたが、そもこの提案にそこまでそそられなければ修正必須ではないのでそこまで重たく考えずとも大丈夫ですので！🙇‍♂️

[reckyy]

@mousu-a まだCIは通ってないですが、先にリプライだけしておきます🙏

具体的なイメージの共有ありがとうございます。 なるほど、 except_unnecessary_mentionsはよさそうですね。 ただ、個人的には以下の理由からメソッド化に踏み切れないです。🥲

• 変数名と処理で十分何をしているのか理解できそう
• テストが増える（このファイルのモデルテストはなさそうですが。。）

ただ、自分が実装しているから（主観的な意見）になってしまっていて、 @mousu-a さんがレビュー時にコンテキストが上がると判断されたならそれは他の方にも当てはまると思います。

そのため、 komagataさんに確認していただく際にこのメソッド化の是非を問う形でもよろしいでしょうか？ 🙇

[mousu-a]

@reckyy

ご検討ありがとうございますー！

テストが増える（このファイルのモデルテストはなさそうですが。。）

なるほどです、テストのことを全く考えていませんでした😅

komagataさんに確認していただく際にこのメソッド化の是非を問う形でもよろしいでしょうか？ 🙇

すみません、よろしければぜひお願いしますー！🙏 ご丁寧な対応ありがとうございます🙇‍♂️

[mousu-a]

@reckyy

approveさせていただきました🙏

[reckyy]

@komagata お疲れ様です。チームメンバーにApproveいただいたのでお手隙の際にご確認お願いいたします。 また、以下の点について意見をお伺いしたいです。

mentionable_without_code = mentionable.gsub(/```.*?```|.*?/m, '')をメソッド化するかどうか

こちらのコードを、extract_login_names_from_mentionsメソッド内に追加しています。 用途としては、コード内のメンションを抽出しないように設けています。 こちらのメソッド化の是非についてお伺いしたいです。 @mousu-a さんとのコミュニケーションをご覧いただいた方が分かりやすいと思うので、お手数ですがそちらをご覧いただきたいです。 https://github.com/fjordllc/bootcamp/pull/7536#discussion_r1527139930 https://github.com/fjordllc/bootcamp/pull/7536#discussion_r1527144046 https://github.com/fjordllc/bootcamp/pull/7536#issuecomment-2001989452 https://github.com/fjordllc/bootcamp/pull/7536#issuecomment-2001993756 https://github.com/fjordllc/bootcamp/pull/7536#issuecomment-2002002228 https://github.com/fjordllc/bootcamp/pull/7536#issuecomment-2002284934

[komagata]

@reckyy (CC: @mousu-a ) ruby本体でも正規表現を定数にすることでその名前で意味を表すことがよくあります。（URI::MailTo::EMAIL_REGEXPとか。）

なので正規表現を変数か定数にするのがいいと思います。CODE_BLOCK_REGEXPとか。

[reckyy]

@komagata お疲れ様です。コメントありがとうございます。

正規表現の定数化は考えていませんでした！今すぐ実装しようと思います。

ただレビュー依頼の前に、他PRでコメントいただいていた内容についてです。

コミットメッセージの先頭に記号を入れるルールは採用しているプロジェクトではよいですが、一人だけ採用すると統一感がなくなって余計にわかりずらくなってしまうので、プロジェクト全体で同意をとってから導入するようにしてください。

コミットメッセージのprefixについてですが、こちらprefixがないコミットメッセージを採用した別PRを再作成したほうがよろしいでしょうか。

他のPR2つでも同様の質問を行なっています。もし3つのPRのうち、一つにご回答いただければ他PRでのこの質問内容に関しては、スルーしていただいて大丈夫です。 🙇 （ただ、コミットメッセージ以外の質問も行なっている場合がありますので、コメントに関してはご確認いただきたいです。）

[reckyy]

@komagata お疲れ様です。修正しましたので、再度ご確認をお願い致します。

[reckyy]

@komagata お疲れ様です。 テスト名を英語に修正したので、ご確認お願いいたします。 このファイルは半分ぐらい日本語のテスト名があったので日本語にしてしまっていました。

テスト名を英語に変更

[reckyy]

@komagata コメントありがとうございます！ 修正しましたので、再度ご確認のほどよろしくお願いいたします。