-
Notifications
You must be signed in to change notification settings - Fork 71
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
入会30日経過した受講生の相談部屋にメッセージを送信する #5959
入会30日経過した受講生の相談部屋にメッセージを送信する #5959
Conversation
0d682e2
to
275e6f0
Compare
@peno022 お疲れさまです〜。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ksmxxxxxx
レビュー依頼いただきありがとうございます!
動作確認問題なかったです🙋♀️
修正内容について1箇所コメントしておりますので、要件ご確認いただけますと幸いです!🙏
/scheduler/daily_controller
肥大化の観点から、/scheduler/send_message_controller
を生やしました
controllerの分け方がこれで問題ないかについては、#5932 のissueで確認中かなと思いますので、私のほうでは問題なしです〜
※
動作確認方法についてですが、説明欄に記載の下記手順(手順4以降)だと、send_messageの処理によってメッセージが送信されたのかどうか確認しづらいかも・・(コメント時刻から確認することはできると思うのですが)と思ったので、
/localhost:3000/scheduler/send_message/にアクセスする(画面は真っ白です)
komagtaでログイン
管理ページから「現役生」タグを選択。**入会 三十郎(thirty)**を探して詳細ページ→相談部屋へ移動
komagataからメッセージがきていることを確認
こちらの手順で確認しました。もし必要であれば説明欄の動作確認手順を適宜修正いただいてもいいかもです🙏
- komagtaでログイン
- 入会30日目のユーザー(thirty)、入会31日以降のユーザー(hatsuno)、入会30日未満のユーザー(otameshi)の相談部屋にメッセージがないことを確認する
- /localhost:3000/scheduler/send_message/にアクセスする(画面は真っ白です)
- 入会30日目のユーザー(thirty)の相談部屋に、komagataからメッセージがきていることを確認する
- 入会31日以降のユーザー(hatsuno)、入会30日未満のユーザー(otameshi)の相談部屋にはメッセージがきていないことを確認する
@peno022 |
e4bd411
to
f51f929
Compare
@peno022 あけましておめでとうございます! |
f51f929
to
d444dc4
Compare
mainにマージされているPull reqがあったのでrebase ed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ksmxxxxxx
あけましておめでとうございます!今年もよろしくお願いします!🐇
年末年始寝込んだりしていて時間があいてしまいすみません🙏
ソースにコメントさせていただきましたので、ご確認お願いします!
それと、追加で1点質問です。
このままリリースすると、リリース後初回のメッセージ送信処理実行時に、その断面で入会30日以降のユーザー全員に一気にメッセージが飛ぶかも?と思ったのですが、そうはならないですかね‥?
3e4fdf6
to
f450128
Compare
@peno022 だいぶお待たせしておりすみません〜 💦
こちらに関してはチーム開発MTGでkomagataさんに伺った方法で対応する必要があるとのことなので、migrateファイルを追加しております。 参考 → DBのデータをmigrateする方法 · fjordllc/bootcamp Wiki 上記が追加になったので、descriptionの確認手順も追加しております。4以降アタリになりますので、合わせてご確認いただけると 🙏 💦 お手数おかけして恐縮ですが、再レビューお願いしていいでしょうか。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ksmxxxxxx
ありがとうございます!確認させていただきました!
(修正箇所も幅広いしデータの更新の考慮もあるし難しい、、おつかれさまです〜〜)
記載いただいた手順で、メッセージ送信の確認ができました。
送信対象の指定と、命名についてコメントさせていただいたので、ご確認お願いします🙏
また、私がdata_migrate gemの理解が浅くて恐縮なんですが、一点質問もさせていただいております。🙇♀️
よろしくお願いします!
db/migrate/20221231043221_add_sent_message_after_thirty_days_to_users.rb
Outdated
Show resolved
Hide resolved
@peno022 お疲れさまです〜 コメントの指摘箇所の他に
何度もお願いしてて恐縮なんですが、どうぞよろしくおねがいします 🙇♀️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ksmxxxxxx
おつかれさまです!レビュー遅くなってしまいすみません😭
私の理解が浅い点あり、いくつか質問コメントさせていただきました・・ご確認お願いできますでしょうか🙇♀️
8bb48df
to
330012e
Compare
894e2e5
to
da75c09
Compare
test/models/user_test.rb
Outdated
@@ -615,4 +615,92 @@ class UserTest < ActiveSupport::TestCase | |||
assert_equal records.first.login_name, 'taikai3' | |||
end | |||
end | |||
|
|||
test 'after twenty nine days registration?' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modelのテストは他のテストに合わせてテストのメソッド名に対してテストを書く感じでいいとおもいます〜。
bootcamp/test/models/user_test.rb
Line 11 in eecf2ee
test '#hibernated?' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4845b57
こちらで修正しました。
@@ -615,4 +615,92 @@ class UserTest < ActiveSupport::TestCase | |||
assert_equal records.first.login_name, 'taikai3' | |||
end | |||
end | |||
|
|||
test '#after_twenty_nine_days_registration?' do | |||
over29days_registered_student = User.create!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このテストでだけテストユーザーを追加するのいいですね!
@ksmxxxxxx レビュー後にすみません、リリース前にちょっと気になったのですが、これって入会後100日経ってるユーザーなど、既存のほとんどのユーザーにメッセージが行く感じですかね? |
@komagata 確認のコメントありがとうございます。 |
かしこまりました〜! |
@komagata
なので
についての回答としては、 「migrationを実行すれば入会100日経過しているユーザーにはメッセージはいかない(はず)」 となります〜 |
@ksmxxxxxx なるほどです、じゃあ大丈夫ですね!ありがとうございます〜! |
@ksmxxxxxx リリースのためにこちらのステージング環境での確認をお願いします〜 |
Issue
概要
:sutudents
)にkomagataさんからコメントが届く仕組みを実装/scheduler/daily_controller
肥大化の観点から、/scheduler/send_message_controller
を生やしましたsent_message_after_thrity_days
がfalse
になっている変更確認方法
feature/send-message-to-students-after-30-days
をローカルに取り込むbin/rails db:migrate
を実行bin/rails db:reset
してDB初期化と、seedデータを取り込むbin/rake data:migrate:up VERSION=20230114032018
を実行(既存ユーザーのsent_message_after_thirty_days
をtrue
にする処理)bin/rails s
でサーバー起動/localhost:3000/scheduler/daily/send_message
にアクセスする(画面は真っ白です)メッセージ
Screenshot