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.

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

Q&Aでベストアンサーが決まったらウォッチしているユーザに通知する #3082

Merged
merged 6 commits into from
Aug 11, 2021

Conversation

rikuba
Copy link
Contributor

@rikuba rikuba commented Aug 3, 2021

Ref: #1776

概要

Q&Aにおいて、ベストアンサーが選ばれたときに「ベストアンサーの回答者」および「その質問をWatchしているユーザ(質問者を除く)」に対して、サイト内通知とメール通知を行います。

また、Discordの通知チャンネルに「質問「質問タイトル」のベストアンサーが選ばれました。」というメッセージを質問のURL付きで投稿します。

サイト内通知

メール通知

開発環境ではコンソールにletter_openerのパスが出力されます。

また、http://127.0.0.1:3000/rails/mailers/notification_mailer/chose_correct_answer からfixturesのデータを使ったプレビューを見ることができます。

Discordへの通知

開発環境ではコンソールにMessage to Discord.が出力されます。

@rikuba rikuba force-pushed the feature/correct-answer-notification branch from ffad446 to c7ca10a Compare August 4, 2021 07:23
@rikuba rikuba marked this pull request as ready for review August 4, 2021 08:41
@rikuba rikuba requested a review from yana-gi August 4, 2021 08:42
@rikuba
Copy link
Contributor Author

rikuba commented Aug 4, 2021

@yana-gi

このPRのレビューをお願いいたします🙏

Copy link
Contributor

@yana-gi yana-gi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

見ました!
テストが細かく書かれていて素晴らしいです〜自分も読んでいて勉強になりました!

2点ほど気になったことがあるのでコメントしたのでご確認お願いします🙏

@@ -20,7 +20,8 @@ class Notification < ApplicationRecord
trainee_report: 10,
moved_up_event_waiting_user: 11,
create_pages: 12,
following_report: 13
following_report: 13,
chose_correct_answer: 14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

他のkindの項目が過去形(か名詞)になっているので、
chosed_correct_answerの方が良さそうです!

Copy link
Contributor Author

@rikuba rikuba Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

現在形がchoose、過去形がchoseになるようです。
(辞書で調べるまですっかり忘れていました😓)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かにそうでした!
こちらの確認不足でした。申し訳ないです🙏

def notify_correct_answer(answer)
question = answer.question
watcher_ids = question.watches.pluck(:user_id)
receiver_ids = [answer.user_id] | watcher_ids - [question.user_id]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここの処理って
通知を受け取るユーザー = 質問をWatchしているユーザー(質問者をのぞく) + (質問をWatchしていない)ベストアンサー回答者
という認識であっていますかね?

質問の回答者は自動的にWatchされるはずなので、わざわざWatchを外している回答者にも通知を送る必要はないかなと思ったのですが、いかがでしょう……!

Copy link
Contributor Author

@rikuba rikuba Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

はい、その認識で間違いありません。
確かにあえてWatchを外しているユーザーに通知を送るのは迷惑そうですね💦
「Watchしているユーザー - 質問者」になるように修正します!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正とテストの変更もありがとうございますー😄

@rikuba
Copy link
Contributor Author

rikuba commented Aug 6, 2021

@yana-gi
レビューありがとうございます🙏(お褒めの言葉もありがとうございます!)
ご指摘いただいた「Watchを外した回答者」の扱いについて修正しました。
chose_correct_answerについてももし違和感などありましたらご指摘ください!

Copy link
Contributor

@yana-gi yana-gi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rikuba
修正ありがとうございますー!
確認しましたのでapproveしますね🙆‍♂️

@rikuba
Copy link
Contributor Author

rikuba commented Aug 7, 2021

@yana-gi
レビューありがとうございました🙇‍♂️

@komagata
このPRのレビューをお願いいたします🙏

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認しました、OKですー🙆‍♂

@machida さんにメール周りのデザイン等確認をお願いしてみてください~

@rikuba
Copy link
Contributor Author

rikuba commented Aug 9, 2021

@komagata
レビューありがとうございました🙇‍♂️

@machida
こちらのメールのデザイン等のご確認をお願いできますでしょうか🙏

@machida
Copy link
Member

machida commented Aug 10, 2021

@rikuba デザイン了解ですー🙆‍♂️

@machida
Copy link
Member

machida commented Aug 11, 2021

@rikuba デザイン確認しましたー!!このままでOKですー

@machida machida removed their assignment Aug 11, 2021
@machida
Copy link
Member

machida commented Aug 11, 2021

では、マージしてしまいます🎉🎉🎉

@machida machida merged commit 1ae7547 into main Aug 11, 2021
@machida machida deleted the feature/correct-answer-notification branch August 11, 2021 08:39
@github-actions github-actions bot mentioned this pull request Aug 11, 2021
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants