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

休会から復帰したら相談部屋へメッセージを投稿する #6755

Merged
merged 7 commits into from
Aug 8, 2023

Conversation

monyatto
Copy link
Contributor

@monyatto monyatto commented Jul 22, 2023

Issue

概要

休会から復帰したら相談部屋にメッセージが投稿されるようにしました。

変更確認方法

  1. feature/send-message-to-comebacked-studentsをローカルに取り込む
  2. 休会からの復帰(http://localhost:3000/comeback/new )にアクセスし、IDkyuukai(休会中のユーザー)の復会手続きを行う
  3. 相談部屋(http://localhost:3000/talks/275641135#latest-comment )にIDkomogataからのメッセージが投稿されているのを確認する

Screenshot

変更前

復会してもメッセージは送られない

変更後

image

@monyatto monyatto force-pushed the feature/send-message-to-comebacked-students branch from d550628 to 64e4a09 Compare July 25, 2023 05:25
@monyatto
Copy link
Contributor Author

@Kassy0220
お疲れさまです!
こちらのレビューをお願いしたいのですが、ご都合いかがでしょうか?
(特に急ぎではなく1~2週間程度で最初のレビューいただけたら嬉しいなくらいです〜)

@monyatto monyatto requested a review from Kassy0220 July 25, 2023 09:49
@Kassy0220
Copy link
Contributor

@monyatto
お疲れ様です🍵

レビュー了解しました!
今ちょっとバタバタしているので、一週間後を目処にレビューさせていただきます🔍

@monyatto monyatto force-pushed the feature/send-message-to-comebacked-students branch from 0ca984d to d365d35 Compare July 27, 2023 04:11
Copy link
Contributor

@Kassy0220 Kassy0220 left a comment

Choose a reason for hiding this comment

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

@monyatto
お疲れ様です🍵

確認しました、コードはほぼ大丈夫だと思います!
細かい箇所で一点修正して欲しい箇所がありますので、ご確認をよろしくお願いいたします🙏

@@ -371,3 +371,5 @@ ja:
send_message:
description: "こんにちは、komagataです。\nご登録から30日ほど経ちますが、学習について困っていることや詰まっている部分などありますでしょうか?\nもしあればお気軽におっしゃっていただければと思います。"
unregistered: 未登録
comeback:
message: "お帰りなさい!!復会ありがとうございます。\n休会中に何か変わったことがあれば、再びスムーズに学び始めることができるように全力でサポートします。何か困ったことや質問があれば、遠慮なくご相談ください。\n\nまたフィヨルドブートキャンプの Discord のサーバーに入室できるように、再度、Doc にある Discord の招待 URL にアクセスをお願いします。\n<https://bootcamp.fjord.jp/practices/129#url>"
Copy link
Contributor

Choose a reason for hiding this comment

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

こちら、末尾に空行の追加をお願いします〜。

あと、ちょっと気になったのが、一つ前のsend_message: description:と整合性を取るなら、次のようにも書けるかなと思いました。

  comeback_message:
    description: "お帰りなさい!!復会ありがとうございます。\n休会中に何か変わったことがあれば、再びスムーズに学び始めることができるように全力でサポートします。何か困ったことや質問があれば、遠慮なくご相談ください。\n\nまたフィヨルドブートキャンプの Discord のサーバーに入室できるように、再度、Doc にある Discord の招待 URL にアクセスをお願いします。\n<https://bootcamp.fjord.jp/practices/129#url>"

ただ、ロケールファイルの書き方に規約があるわけではなさそうなので、このままでも良いかなと思います!

send_message: :description:の書き方も、特に何か規約に沿っているわけではなさそうです。
入会30日経過した受講生の相談部屋にメッセージを送信する by ksmxxxxxx · Pull Request #5959 · fjordllc/bootcamp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

send_message: description:がかなり汎用的な名前のため、今回の命名で整合性を取るのは難しいと判断して一旦このままにしました。次の開発MTGでsend_message: description:の名前の変更も考慮に新しいissue立てるべきか聞いてみます🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

了解しました、はるまきさんの判断に賛成です👍

次回の開発MTGでの確認よろしくお願いします🙏
これから相談部屋に送るメッセージが増える可能性もあるので、名前の変更はあっても良いのではないかと思います。

もし変更するならば、send_message:をメッセージの一括りとして、その下に個別メッセージを追加するといった感じでしょうか。

send_message:
  followup: "こんにちは、komagataです。\nご登録から30日ほど経ちますが、..."
  comeback: "お帰りなさい!!復会ありがとうございます。\n休会中に何か変わったことがあれば、..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちら開発MTGで相談して以下の形でこのPR内で実装することになりました。

talk:
    followup: 30日経過後のメッセージ
    comeback: 復会後のメッセージ

"またフィヨルドブートキャンプの Discord のサーバーに入室できるように、再度、Doc にある Discord の招待 URL にアクセスをお願いします。\n" \
'<https://bootcamp.fjord.jp/practices/129#url>'
assert_equal users(:komagata).id, comment.user_id
assert_equal description, comment.body
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらのテストに関して、さらに「hajimeさんの相談部屋にコメントが飛んでいる」ことを確認しても良いかなと考えました。

アサーションを追加するかどうかは、はるまきさんにお任せします〜。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちら相談部屋が正しいか確かめるアサーションも追加しました!

Copy link
Contributor

@Kassy0220 Kassy0220 left a comment

Choose a reason for hiding this comment

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

@monyatto
ご対応ありがとうございました!
LGTMですので、Approveさせていただきます〜🙌

@monyatto
Copy link
Contributor Author

@Kassy0220
レビューありがとうございます!

@komagata
チームメンバーからApproveいただきましたのでレビューお願いいたします🙏

assert_difference 'Comment.count', 1 do
hajime.create_comebacked_comment
end
comment = Comment.last
Copy link
Member

Choose a reason for hiding this comment

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

基本的に複数のテストが並行して走るのでlastだと同時に動いた時に落ちてFlakyなテストにならないかが気になりました。

@monyatto monyatto force-pushed the feature/send-message-to-comebacked-students branch from eba7c7e to 84c3232 Compare August 3, 2023 09:19
@monyatto
Copy link
Contributor Author

monyatto commented Aug 3, 2023

@komagata
こちらご指摘部分修正しましたので確認お願いします〜

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 b7e2631 into main Aug 8, 2023
@komagata komagata deleted the feature/send-message-to-comebacked-students branch August 8, 2023 07:37
@github-actions github-actions bot mentioned this pull request Aug 8, 2023
1 task
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