定期イベントでメンション付きのコメントをしたときにメール通知とメッセージが不適切になるバグを修正

[hagiya0121]

Issue

定期イベントでメンション付きのコメントをした場合、メールと通知のメッセージが不適切 #6365

概要

定期イベントのページでメンション付きのコメントをしたときにメール通知とメッセージが不適切になるバグを修正 しました。
issueに書かれているようにRegularEventクラスに対応するメンションメッセージがなかったので追加して対応しました。

変更確認方法

1. bug/fix-event-mention-notificationをローカルに取り込む
2. foreman start -f Procfile.devでサーバーを立ち上げ
3. 定期イベントページにアクセス
   http://localhost:3000/regular_events/459650222
4. 定期イベントページでメンション付きのコメントをする
5. メンションされたユーザーでログイン
6. 定期イベント「開発MTG」へのコメントで<ユーザー名>さんからメンションがきました。という通知が来ていることを確認
7. letter_opennerにアクセス
   http://localhost:3000/letter_opener/
8. subjectが[FBC] komagataさんの定期イベント「開発MTG」に<ユーザー名>さんがコメントしました。というメールが来ていることを確認

Screenshot

変更前

• 通知

---

• メール

---

変更後

• 通知

---

• メール

---

[hagiya0121]

@nakamu-kazu222
お疲れ様です。
こちらのPRのレビューをお願いしたいです🙏
ご都合が悪いときは遠慮なくおっしゃってください🙇

[nakamu-kazu222]

@hagiya0121

お疲れ様です！
申し訳ございませんが、現在都合がつかず、レビューが難しい状況です。

また別の機会にご協力できるよう努めますので、引き続きよろしくお願いいたします。

[hagiya0121]

@nakamu-kazu222
全然大丈夫です🙇

[hagiya0121]

@naokinaokiboo
お疲れ様です。
こちらのPRのレビューをお願いしたいです🙏
ご都合が悪いときは遠慮なくおっしゃってください🙇

[naokinaokiboo]

@hagiya0121
申し訳ありません。💦
直近、全く時間が取れておらずレビュー対応が難しい状況です。🙇‍♂️

[hagiya0121]

@naokinaokiboo
大丈夫です🙇

[hagiya0121]

@kyokucho1989
お疲れ様です。
こちらのPRのレビューをお願いしたいです🙏
ご都合が悪いときは遠慮なくおっしゃってください🙇

[kyokucho1989]

@hagiya0121
了解しました！
一週間ほどお待ちください

[kyokucho1989]

@hagiya0121

おはようございます。動作確認しました。問題ありません。
ただ、テストを追加した方が良いかと思います。

bootcamp/test/system/announcements_test.rb

Line 72 in 6d287a4

test 'create announcement with notification' do

ここの箇所の要領で。

1. イベントページに遷移
2. 任意の人にメンションしてコメント
3. メンションされた人でログイン
4. 通知ページへアクセス
5. 所望の通知文になっているか確認

test/system/events_test.rb へ書くと良いかと思います。

[hagiya0121]

@kyokucho1989
素早いレビューありがとうございます🙏
質問があるのですが、追加するテストはイベントのメンション通知だけでしょうか？
target_of_commentメゾッド内で生成される他のメッセージもテストした方がいいのか悩んで質問しました。

[kyokucho1989]

@hagiya0121
一旦は定期イベントのみで問題ないかと思います。
今回の実装で変更になる部分がそこなので。

同じファイルに記載にするのであれば、特別イベントのテストを追加させてもいいかもしれません。

[hagiya0121]

@kyokucho1989
お疲れ様です！
テストを追加しましたので、ご確認お願いします🙇

[kyokucho1989]

問題ありません。Approveします。

[hagiya0121]

@kyokucho1989
お忙しいなかレビューありがとうございます🙏

[hagiya0121]

@komagata
メンバーのレビューが終わりましたので、レビューお願いします🙏

[komagata]

@okuramasafumi こちらのレビューお願いできればありがたいです〜

[okuramasafumi]

内容は大丈夫だと思います、テストも書いてあって良い感じです！

ここからは設計の提案になります。PR自体は承認しているので、さらに改善してみたいと思ったら試してみてください。

---

今回のバグの原因は単純に今回追加された行の書き忘れということだと思いますが、ある意味では書き忘れが許容されるような設計になっているということです。
target_of_commentメソッドはnilを返す可能性があります。これを改良してnilを返す代わりに例外を上げるようなコードを考えます。例えば：

def target_of_comment(commentable_class, commentable)
  commentable.target_of_comment
end

module Commentable
  def target_of_comment(commentable_class, commentable)
    raise "Define it!"
  end
end

class RegularEvent
  def target_of_comment(commentable_class, commentable)
    "定期イベント「#{title}」
  end
end

この実装では、共通モジュールであるCommentableでメソッドが定義されており、そこでは例外が上がります。この例外が出ないようにするにはtarget_of_commentメソッドを各クラスで定義する必要があります。
これは一例ですが、今回のようなバグが出たときは設計を見直すチャンスと言えるかもしれません。このPRでやらなくても全然いいのですが、設計について考えるヒントになれば幸いです。

[hagiya0121]

@okuramasafumi
レビューありがとうございます🙏
バグを直すのに頭がいっぱいで、なぜそうなってしまったかは考えられていなかったので勉強になりました。