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

ベストアンサーなし通知をactive_delivery化する #6578

Conversation

ogawa-tomo
Copy link
Contributor

@ogawa-tomo ogawa-tomo commented May 29, 2023

Issue

概要

質問に対する最後の回答から1週間が経過し、ベストアンサーが決まっていない場合、質問を投稿した人にサイト内通知とメール通知が飛びます。
その通知をactive_delivery化しました。

変更確認方法

  1. feature/replace-notification-on-no-correct-answer-with-active-deliveryをローカルに取り込む
  2. Railsコンソールで、適当なユーザー(ここではhatsuno)で2週間前の質問を2つ作成する
> user = User.find_by(login_name: 'hatsuno')
> time = 2.week.ago
> question1 = Question.create!(title: 'test1', description: 'test1', user: user, practice: Practice.first, created_at: time, updated_at: time, published_at: time)
> question2 = Question.create!(title: 'test2', description: 'test2', user: user, practice: Practice.first, created_at: time, updated_at: time, published_at: time)
  1. さらに、上記の1つ目の質問に6日前の回答を、2つ目の質問に7日前の回答をつける
> day6 = 6.day.ago
> question1.answers.create!(description: '6日前の回答', user: user, created_at: day6, updated_at: day6)
> day7 = 7.day.ago
> question2.answers.create!(description: '7日前の回答', user: user, created_at: day7, updated_at: day7)
  1. Q&Aページにアクセスし、上記の質問・回答が正しく作られていることを確認する
    image
    image

  2. http://localhost:3000/scheduler/daily/notify_certain_period_passed_after_last_answer にアクセス

  3. 質問を作成したユーザーでアクセスし、2つ目の質問についてのみ通知が来ていることを確認
    image

  4. http://localhost:3000/letter_opener/ にアクセスし、2つ目の質問についてのみメール通知が飛んでいることを確認
    image

やったこと

  • NotificationFacadeをActivityNotifierに置き換え
  • NotificationMailerをActivityMailerに置き換え
  • Mailerのプレビュー画面を追加

変更前

flowchart TD
  NotifyCertainPeriodPassedAfterLastAnswerController -- notify_certain_period_passed_after_last_answer --> Question
  Question -- no_correct_answer --> NotificationFacade
  NotificationFacade -- no_correct_answer --> NotificationMailer:::orange
  NotificationFacade:::orange -- no_correct_answer --> ActivityNotifier

  classDef orange fill:#f96
Loading

変更後

flowchart TD
  NotifyCertainPeriodPassedAfterLastAnswerController -- notify_certain_period_passed_after_last_answer --> Question
  Question -- "notify(:no_correct_answer)" --> ActivityDelivery
  ActivityDelivery -. no_correct_answer .-> ActivityMailer:::orange
  ActivityDelivery:::orange -. no_correct_answer .-> ActivityNotifier

  classDef orange fill:#f96
Loading

参考情報

ベストアンサーなし通知を導入したPR

通知関連の参考リンク

active_delivery化の参考PR

(他にもactive_delivery化のPRは多数あり)

@ogawa-tomo ogawa-tomo changed the title ベストアンサー決定通知をactive_delivery化する ベストアンサーなし通知をactive_delivery化する May 29, 2023
@ogawa-tomo ogawa-tomo force-pushed the feature/replace-notification-on-no-correct-answer-with-active-delivery branch 2 times, most recently from 5b7505b to 55043dd Compare May 29, 2023 11:54
@ogawa-tomo ogawa-tomo marked this pull request as ready for review May 29, 2023 13:09
@ogawa-tomo
Copy link
Contributor Author

@YukiWatanabe824 こちら、よろしければレビューお願いできますでしょうか?

@YukiWatanabe824
Copy link
Contributor

了解しました!
最長1週間ほどでレビューいたします😄

@ogawa-tomo ogawa-tomo force-pushed the feature/replace-notification-on-no-correct-answer-with-active-delivery branch from dacf750 to 63861eb Compare May 30, 2023 11:57
@ogawa-tomo ogawa-tomo self-assigned this May 30, 2023
Copy link
Contributor

@YukiWatanabe824 YukiWatanabe824 left a comment

Choose a reason for hiding this comment

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

確認しました!
良いと思いますのでapproveいたします😄👍!

@ogawa-tomo ogawa-tomo force-pushed the feature/replace-notification-on-no-correct-answer-with-active-delivery branch from 63861eb to 18685d3 Compare June 23, 2023 11:49
@ogawa-tomo
Copy link
Contributor Author

@komagata こちら、受講生のレビューが完了し、コンフリクトを解消しましたので、レビューをお願いします:bow:

@ogawa-tomo ogawa-tomo requested a review from komagata June 23, 2023 12:08
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です〜🙆‍♂️

@komagata komagata merged commit e7fbc23 into main Jun 28, 2023
@komagata komagata deleted the feature/replace-notification-on-no-correct-answer-with-active-delivery branch June 28, 2023 19:41
@github-actions github-actions bot mentioned this pull request Jun 28, 2023
21 tasks
@ogawa-tomo
Copy link
Contributor Author

@komagata @machida
本PRがリリースブランチにマージされていますので、動作確認方法について質問させてください:pray:
これの動作確認をするためには、質問に対する「ちょうど一週間前の回答」を用意する必要があります。

ステージング環境での動作確認について

#5832 で動作確認のために一週間前の回答をfixtureファイルに用意されているようですが、これを使うためにはステージング環境のDBがリセットされたその日のうちに動作確認を行う必要があります。現在、リセットされてから数日経ってしまいましたので、動作確認ができない状態になってしまっています。DBをリセットしていただくことは可能でしょうか?

本番環境での動作確認について

この機能が実装された際のIssueにあるこちらのコメント#5451 (comment) を見ると、実際に本番環境に質問・回答を投稿して一週間待っていたようですが、今回もその方法でよいでしょうか?(テスト用の質問が本番環境に一週間も残っていいのかなと若干気になってしまったので確認です)

@komagata
Copy link
Member

komagata commented Jul 1, 2023

@ogawa-tomo

ステージング環境での動作確認について

ステージング環境はデプロイ時にDBがリセットされるようになっていますのでそちらを利用していただければと思います。

本番環境での動作確認について

はい。
おっしゃるやり方で大丈夫です。

@ogawa-tomo
Copy link
Contributor Author

📝 ステージング環境での動作確認手順

  1. kimuraでログイン
  2. 登録情報変更し、メールアドレスを自分のメールに切り替える
  3. 「1週間前の質問」(https://bootcamp-staging.fjord.jp/questions/700755356 )の最後の回答からちょうど1週間(7日間)が経過していることを確認する
    image
    4.ブラウザでhttps://bootcamp-staging.fjord.jp/scheduler/daily/notify_certain_period_passed_after_last_answer?token=test にアクセス
  4. 「ベストアンサーがまだ選ばれていません」というメールが来ているのを確認
  5. 「ベストアンサーがまだ選ばれていません」というWeb通知が来ているのを確認

@ogawa-tomo
Copy link
Contributor Author

以下の通り、web通知とメール通知が来ることをステージング環境で確認した。
image

image

@ogawa-tomo
Copy link
Contributor Author

ogawa-tomo commented Aug 31, 2023

📝 8/31
本番環境での動作確認に失敗したが、ステージングで再度動作確認をしたところ正常に動作している。
image
image

9/21 追記
ステージングで再度動作確認すると、「aaa」という通知が来た。どういうことだ???→ページャーの動作確認のために連続投稿していたとのこと。
となると、現在はステージングでも動作していない・・・

10/6追記
ステージングでの動作確認に成功。

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.

3 participants