bootcamp icon indicating copy to clipboard operation
bootcamp copied to clipboard

最後の回答から1週間経過したら質問を投稿した人に通知されるようにした

Open keiz1213 opened this issue 2 years ago • 6 comments

issue

  • #5451

概要

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

背景

質問した人がベストアンサーを決め忘れ、closeされないまま未解決の質問として残る問題が背景にあります。

変更確認の準備

定期イベントのdiscord通知をコメントアウトする

変更を確認する際に、曜日によっては定期イベントのdiscord通知が動いてしまうので、コメントアウトしてください。

# app/controllers/scheduler/daily_controller.rb

class Scheduler::DailyController < SchedulerController
  def show
    User.notify_to_discord
    User.retired.find_each do |retired_user|
      if retired_user.retired_three_months_ago_and_notification_not_sent?
        User.admins.each do |admin_user|
          Notification.three_months_after_retirement(retired_user, admin_user)
          NotificationFacade.three_months_after_retirement(retired_user, admin_user)
          retired_user.update!(notified_retirement: true)
        end
      end
    end

   # if RegularEvent.tomorrow_events.present?
    #  RegularEvent.tomorrow_events.each do |regular_event|
      #  NotificationFacade.tomorrow_regular_event(regular_event)
     # end
  #  end

    Question.notify_of_pending if Question.not_solved_and_a_week_has_passed.present?
    head :ok
  end
end

変更確認方法

  1. feature/notify-after-one-week-from-last-answerをローカルに取り込む
  2. rails cで2週間前の質問を2つ作成する
# user_id → kimuraのid
# practice_id → Terminalの基礎を覚える のid
# 2つ作成してください(自分は2つ目はtitleをtest2としました)

Question.create!(title: "test", description: "test", user_id: 991528156, created_at: Time.current.ago(2.week), updated_at: Time.current.ago(2.week), published_at: Time.current.ago(2.week), practice_id: 198065840)
  1. rails sでサーバー起動

  2. kimuraでログイン

  3. http://localhost:3000/users/991528156/questions にアクセスする _development__kimuraの質問一覧___FBC_🔊

  4. 作成した2つの質問の詳細ページにアクセスしそれぞれのidを確認する _development__test___FBC_🔊

  5. rails cで、1つ目の質問に現在から6日前の日付で回答を作成する

Answer.create!(description: "6日前", user_id: 991528156, question_id: 1037106415, created_at: Time.current.ago(6.day), updated_at: Time.current.ago(6.day))
_development__test___FBC_🔊
  1. http://localhost:3000/scheduler/daily にアクセスし、ダッシュボードに戻って通知が来ていないことを確認する
_development__ダッシュボード___FBC_🔊
  1. rails cで、2つ目の質問に現在から7日前の日付で回答を作成する
Answer.create!(description: "7日前", user_id: 991528156, question_id: 1037106416, created_at: Time.current.ago(1.week), updated_at: Time.current.ago(1.week))
_development__test2___FBC_🔊
  1. http://localhost:3000/scheduler/daily にアクセスし、ダッシュボードに戻って通知が来ていることを確認する _development__ダッシュボード___FBC_🔊

  2. http://localhost:3000/letter_opener/ にアクセスしメール通知が来ていることを確認する

_development__LetterOpenerWeb_🔊

keiz1213 avatar Sep 24 '22 18:09 keiz1213

@pachikuriii

お疲れさまです!こちらレビュー頂いてもよろしいでしょうか?全く急いでないのでいつでも大丈夫です😄ご都合が合わない等ありましたらお気軽にご連絡ください😄

keiz1213 avatar Oct 04 '22 03:10 keiz1213

@keiz1213 おつかれさまです〜☕️!レビューのご依頼ありがとうございます〜! 2,3日以内に完了できれば…と思っているのですが、そんなスケジュール感でもよければ 是非レビューさせていただけると嬉しいです💪🏻

pachikuriii avatar Oct 04 '22 07:10 pachikuriii

@pachikuriii 2〜3日以上かかっても全然大丈夫ですのでよろしくお願いします!🙏

keiz1213 avatar Oct 04 '22 08:10 keiz1213

@pachikuriii

とても丁寧なレビューありがとうございます!😄そして修正が遅くなってすみません🙏 ご提案頂いた内容を参考に修正してみましたので、お手すきの際にご確認お願いいたします😄 全然急いでないのでゆっくりで大丈夫です👍

keiz1213 avatar Oct 09 '22 09:10 keiz1213

@pachikuriii 確認ありがとうございます!

a_week_after_last_answer?メソッドを修正しました

@sinsoku さんからアドバイスいただきまして、a_week_after_last_answer?メソッドの不備に気づき、修正しました・・🙏 修正前の条件式だと、「時刻」を考慮しておらず実際の本番環境では期待した動きにならないことがわかりました。なので単純にdateオブジェクトとして比較するように書き直しました。今まで開発環境、テストで問題にならなかったのは、全てのテストデータの作成時刻が00:00で揃っていたためです。

この修正によって変更確認方法を修正しました。今まではコンソールで作成頂いていた質問・回答の作成時刻を Date.current.agoで作成していたものをTime.current.agoで作成して確認するようにしました。こうすることで作成時刻が00:00とならず、その時の時刻で作成され動作確認が正しく行われます。

大変お手数おかけしますが、お時間があります時に再度確認お願いします🙏

@sinsoku

ご指摘いただいたことで不備に気づくことができました。ありがとうございます!

keiz1213 avatar Oct 13 '22 03:10 keiz1213

@pachikuriii 確認ありがとうございます!とても丁寧にレビューいただき勉強になりました😄そして少しでもお役に立てたなら幸いです😄お互いアドバイスし合いながらチーム開発乗り切って行きましょう〜!

@komagata

こちらチームの方からapproveいただきましたのでご確認お願いいたします!

keiz1213 avatar Oct 15 '22 10:10 keiz1213

@komagata コンフリクトの修正完了しました。mainにおいてdaily_controllerに変更が加えられていたためその変更と統一性を持たせるため1点変更を加えました。

以前までは通知処理をQuestionのクラスメソッドで定義しそれをdaily_contorollerで呼び出す形でしたが、今回の変更に伴いそのクラスメソッドを削除し、daily_contoroller内のプライベートメソッドで定義して呼び出すように変更しました。

度々申し訳ありませんがご確認お願いいたします。

keiz1213 avatar Nov 09 '22 15:11 keiz1213

@keiz1213

以前までは通知処理をQuestionのクラスメソッドで定義しそれをdaily_contorollerで呼び出す形でしたが、今回の変更に伴いそのクラスメソッドを削除し、daily_contoroller内のプライベートメソッドで定義して呼び出すように変更しました。

おっと、controllerのprivateメソッドで処理するのはとりあえずな感じのやり方で、モデルクラスのメソッドにする方が望ましい感じです〜(Fat Controller)

komagata avatar Nov 11 '22 06:11 komagata

@komagata

なるほどです。Fat Controllerという考え方があることは知りませんでした。通知処理をQuestionクラスで定義し、それをdaily_controllerから呼び出すように修正しましたのでご確認ください。

keiz1213 avatar Nov 11 '22 08:11 keiz1213

@keiz1213

なるほどです。Fat Controllerという考え方があることは知りませんでした。通知処理をQuestionクラスで定義し、それをdaily_controllerから呼び出すように修正しましたのでご確認ください。

Controllerはモデルのメソッドを呼び出したりといったControlだけをするのが原則になっています。 ControllerをFatにせず、Modelに処理を移すのが第一で、それをすることによってModelがFatになってしまう「Fat Model」が一般的に問題になっています。

komagata avatar Nov 14 '22 14:11 komagata

@komagata

時間関連のテストは実行日時やタイミングによってFlakyなテストになりやすいので、travel_toなどを使って時間を固定してやるのがいいと思います〜

なるほど、勉強になります。 ご指摘いただいた点を参考に修正致しました。ご確認お願いいたします。

keiz1213 avatar Nov 19 '22 06:11 keiz1213