分報チャンネル作成後、生成されたチャンネルのURLをユーザーのプロフィールに自動で設定 #7564

Takuya-Sakai91 · 2024-03-20T15:13:31Z

Issue

分報チャンネルを自動で作成する際にユーザーの分報URLも自動で登録されてほしい #6907

概要

ユーザーの新規登録時に分報チャンネルを自動で作成した際に、プロフィールページの分報URLも自動で入力されるようにしました。また、退会時には分報URLが削除されるようにしました。

変更確認方法

下記PRのディスクリプションにある「変更確認方法」「準備」の手順に従い、テスト用Discordサーバーの作成、Discord Botの作成、環境変数の設定を行う
- 退会時に分報チャンネルを自動で削除する #6626
feature/auto-fill-times-url-on-registrationをローカルに取り込む
bin/rails db:seedを実行し、初期データを追加
bin/rails sでサーバを立ち上げる

分報チャンネル作成後、生成されたチャンネルのURLがユーザーのプロフィールページに自動で設定されているか確認する

ログアウトした状態で参加登録ページから適当な名前で参加登録する
クレジットカード番号にはテスト用の番号を入力する
- https://stripe.com/docs/testing?locale=ja-JP#cards
テスト用のDiscordサーバーに分報チャンネルが作成されていることを確認する
登録したユーザーでログイン
ユーザープロフィールページの分報のリンクから、作成した分報チャンネルに遷移することを確認する

_development__jidouchanelurlsakusei___FBC

登録情報変更ページの「分報URL」にDiscordの分報チャンネルのURLが作成されていることを確認する

ユーザーが自ら退会するときの動作確認

ログアウトした状態で参加登録ページから適当な名前で参加登録する
- クレジットカード番号にはテスト用の番号を入れる
  - https://stripe.com/docs/testing?locale=ja-JP#cards
テスト用のDiscordサーバーに分報チャンネルが作られていることを確認する
先ほど登録したユーザーでログインし、画面右上のユーザーメニューから「退会手続き」を選択して退会する
Discordから分報チャンネルが削除されていることを確認する
Railsコンソールでユーザーの分報チャンネルのURLが削除されているかを確認する

ユーザーを検索

> user = User.find_by(login_name: 'jidouchanelurlsakusei')

検索したユーザーのdiscord_profileに関連付けられたtimes_urlを確認

> user.discord_profile.times_url

=> nil

管理ページから退会させるときの動作確認

ログアウトした状態で参加登録ページから適当な名前で参加登録する
- クレジットカード番号にはテスト用の番号を入れる
  - https://stripe.com/docs/testing?locale=ja-JP#cards
テスト用のDiscordサーバーに分報チャンネルが作られていることを確認する
komagata（管理者）でログインしなおす
右上のユーザーメニューから管理ページに遷移し、ユーザー一覧ページから先ほど作成したユーザーを探し、表右端の「操作」からユーザー登録情報変更画面に遷移する
ユーザーステータスを「退会済」に変更して「更新する」

Discordから分報チャンネルが削除されていることを確認する
Railsコンソールでユーザーの分報チャンネルのURLが削除されているかを確認する

ユーザーを検索

> user = User.find_by(login_name: 'jidouchanelurlsakusei')

検索したユーザーのdiscord_profileに関連付けられたtimes_urlを確認

> user.discord_profile.times_url

=> nil

Takuya-Sakai91 · 2024-03-26T14:58:50Z

@goruchanchan
お疲れ様です！こちらお手数ですがレビューいただけますでしょうか？
ご確認よろしくお願いします🙏

goruchanchan · 2024-03-26T15:19:20Z

@natsuto6 お疲れ様です！レビュー依頼ありがとうございます！１週間以内に確認いたしますので、今しばらくお待ちください🙇‍♂️

goruchanchan · 2024-03-30T14:42:38Z

@natsuto6 お疲れ様です！動作確認できました！問題ないと思います！
ちょっとしたことですが、分報チャネルのURL削除の確認で、『ユーザ自ら退会時は rails c』、『管理ページから退会させるときはユーザ登録情報変更画面』としているのは何か理由があるのでしょうか？理由内容であれば揃えてしまってもいいかなと思いました🙇‍♂️

また、コードレビューについてですが、main が更新されたため差分が出ているように思い、着手できていないです🙇‍♂️
加えて、コンフリクトも発生しているようですので、お手数ですがコンフリクト対応をお願いいたします🙇‍♂️

Takuya-Sakai91 · 2024-03-31T00:34:19Z

@goruchanchan
ご確認いただきありがとうございます！

ちょっとしたことですが、分報チャネルのURL削除の確認で、『ユーザ自ら退会時は rails c』、『管理ページから退会させるときはユーザ登録情報変更画面』としているのは何か理由があるのでしょうか？

こちらですが、画面からの確認を含めて実際のUIをを通じた動作確認を行なっても良いかなという意図で記載してました。ただ、ご指摘の通りより確実に検証できる方に統一した方がいいかなと思ったのと、レビューも煩雑になるのでRailsコンソールからの確認に修正してます🙇‍♂️再度お手数ですがご確認よろしくお願いします。

goruchanchan

@natsuto6 お疲れ様です！コード確認いたしました🙇‍♂️お手すきの時にご確認よろしくお願いします🙇‍♂️

goruchanchan · 2024-03-31T13:28:46Z

test/models/times_channel_creator_test.rb

+      TimesChannelCreator.new.call({ user: })
+    end
+
+    assert_equal '1234567890123456789', user.discord_profile.times_id


URL が期待通りかの確認で十分かと思ったんですが、分報 ID のアサートも取る理由を教えて欲しいです🙇‍♂️

URLの確認だけだとtimes_idが正確に設定されているかを直接的に確認できないので、プロセス全体の正確性を保証する意味で記載してました🙏

goruchanchan · 2024-03-31T13:39:07Z

test/models/times_channel_creator_test.rb

+    end
+
+    assert_equal '1234567890123456789', user.discord_profile.times_id
+    expected_url = "https://discord.com/channels/#{ENV['DISCORD_GUILD_ID']}/1234567890123456789"


今回更新いただいたのが def call(payload)メソッドなので、テスト自体も
test '#call'の中に追加する形でもいいかなと思いましたが、いかがでしょうか？

こちらおっしゃる通りでしたので、test #callに記載しました🙏

goruchanchan · 2024-03-31T14:41:39Z

test/integration/discord/users_controller_test.rb

@@ -36,6 +36,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
      assert_redirected_to root_url

      student = User.find_by(login_name: 'Piyopiyo-student')
+      student.create_discord_profile unless student.discord_profile


こちらなんですが、参考までに以下を教えていただけると助かります🙇‍♂️
Discord::TimesChannel.stub(:new, ->(_) { ValidTimesChannel.new }) doで、用意したスタブ（ValidTimesChannel）を利用しているように見えるのですが、ここでIDの代入が必要になった理由などを教えていただきたいです🙇‍♂️
少し調べたのですが、わからずでした😓

元々テストが落ちていました。

ログ

Failure:
Discord::UsersControllerTest#test_POST_create_by_trainee [/home/circleci/project/test/integration/discord/users_controller_test.rb:70]:
Expected nil to not be nil.

rails test test/integration/discord/users_controller_test.rb:42

[MinitestRetry] retry 'test_POST_create_by_student' count: 1, msg: Expected nil to not be nil.
[MinitestRetry] retry 'test_POST_create_by_student' count: 2, msg: Expected nil to not be nil.
[MinitestRetry] retry 'test_POST_create_by_student' count: 3, msg: Expected nil to not be nil.
F

Failure:
Discord::UsersControllerTest#test_POST_create_by_student [/home/circleci/project/test/integration/discord/users_controller_test.rb:39]:
Expected nil to not be nil.

rails test test/integration/discord/users_controller_test.rb:7

ValidTimesChannelのidがnilであるのを解消するために、テスト内で
ValidTimesChannelのスタブを使用することで、実際にDiscord APIにリクエストを送ることなく、この挙動をテスト環境内で再現してます。
times_idを明示的に設定しているのは、Discord::TimesChannelのスタブが返すidを使用して、discord_profileのtimes_idが期待通りに設定されることを確認するためです。

回答ありがとうございます！自分もそうなのかと思い、main ブランチで試した結果、下記の結果となりテストが通るようでした🤔

テスト結果

% git status On branch main Your branch is up to date with 'origin/main'. nothing to commit, working tree clean % rails test test/integration/discord/users_controller_test.rb Running via Spring preloader in process 19387 Run options: --seed 13748 # Running: ... [Minitest::CI] Generating test report in JUnit XML format... Finished in 2.142988s, 1.3999 runs/s, 4.1997 assertions/s. 3 runs, 9 assertions, 0 failures, 0 errors, 0 skips

実際に、byebug を仕込んで、分報 ID を確認すると、main では ID 代入せずとも、スタブにより分報 ID が設定されているでした🤔

byebug での times_id の確認結果

39: byebug => 40: assert_not_nil student.discord_profile.times_id 41: end 42: 43: test 'POST create by trainee' do 44: Discord::TimesChannel.stub(:new, ->(_) { ValidTimesChannel.new }) do (byebug) student.discord_profile #<DiscordProfile id: 1066908204, user_id: 1060516511, account_name: nil, times_url: nil, times_id: "fake_times_snowflake_0123456789", created_at: "2024-04-01 22:25:35.081484000 +0900", updated_at: "2024-04-01 22:25:35.261921000 +0900">

ValidTimesChannel を用いて分報インスタンスが作られていれば、従来のままでテストも通るのかなと思っていたので、今回の変更で何か変わってないかが少し気になっています🙇‍♂️

Takuya-Sakai91 · 2024-04-01T08:14:47Z

@goruchanchan
早々のご確認ありがとうございます！🙇‍♂️
ご指摘いただいていた箇所を修正したので、お手数ですが再度ご確認をお願いします🙏

goruchanchan

@natsuto6 お疲れ様です！ご対応とご回答ありがとうございます！参考になります👀２点コメントさせていただきましたので、お手すきの時にご確認お願いいたします🙏

goruchanchan · 2024-04-01T13:32:41Z

test/integration/discord/users_controller_test.rb

@@ -36,6 +36,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
      assert_redirected_to root_url

      student = User.find_by(login_name: 'Piyopiyo-student')
+      student.create_discord_profile unless student.discord_profile


回答ありがとうございます！自分もそうなのかと思い、main ブランチで試した結果、下記の結果となりテストが通るようでした🤔

テスト結果

% git status On branch main Your branch is up to date with 'origin/main'. nothing to commit, working tree clean % rails test test/integration/discord/users_controller_test.rb Running via Spring preloader in process 19387 Run options: --seed 13748 # Running: ... [Minitest::CI] Generating test report in JUnit XML format... Finished in 2.142988s, 1.3999 runs/s, 4.1997 assertions/s. 3 runs, 9 assertions, 0 failures, 0 errors, 0 skips

実際に、byebug を仕込んで、分報 ID を確認すると、main では ID 代入せずとも、スタブにより分報 ID が設定されているでした🤔

byebug での times_id の確認結果

39: byebug => 40: assert_not_nil student.discord_profile.times_id 41: end 42: 43: test 'POST create by trainee' do 44: Discord::TimesChannel.stub(:new, ->(_) { ValidTimesChannel.new }) do (byebug) student.discord_profile #<DiscordProfile id: 1066908204, user_id: 1060516511, account_name: nil, times_url: nil, times_id: "fake_times_snowflake_0123456789", created_at: "2024-04-01 22:25:35.081484000 +0900", updated_at: "2024-04-01 22:25:35.261921000 +0900">

ValidTimesChannel を用いて分報インスタンスが作られていれば、従来のままでテストも通るのかなと思っていたので、今回の変更で何か変わってないかが少し気になっています🙇‍♂️

goruchanchan · 2024-04-01T13:39:23Z

test/models/times_channel_creator_test.rb

@@ -46,6 +48,16 @@ class TimesChannelCreatorTest < ActiveSupport::TestCase
    assert_not user.student_or_trainee?
  end

+  test '#call creates times_url' do


test '#call' だけで十分かなと思いますが、いかがでしょうか？🙇‍♂️

既にtest '#call'という名前のテストケースが存在しているので、それぞれのテストケースが何をテストしているのかを区別するために今のままで良いのかなと思いますがいかがでしょう？

伝え方が悪くて申し訳ないです🙇‍♂️#call のテストに統合してしまってもいいじゃないかなと思った次第です🙇‍♂️

call メソッドは、今回の変更にて time_id に加えて times_url も設定されることになると思うので、その内容を追加するといいのかなと思っています🙇‍♂️

また、#call creates times_url としてテストする場合、ValidTimesChannel によって正しい設定をするだけで終わってしまっているので、当該の正しい設定がされているかの評価があった方がいいと思いました🤔

Takuya-Sakai91 · 2024-04-06T00:22:42Z

@goruchanchan
test/integration/discord/users_controller_test.rbのテストがmainブランチでは通るが、プルリクエストの変更後に失敗するという件について、app/models/times_channel_creator.rbで生成しているtimes_urlが正常に生成されていませんでした。原因は以下です

ValidTimesChannelクラスのidメソッドが文字列を返していたこと（これは期待される挙動ですが、テストの文脈で正しく扱われていませんでした）
DISCORD_GUILD_IDの環境変数を正しく取得できていなかったこと

なのでテスト内でValidTimesChannelクラスのidメソッドが数値としての文字列を返すように修正し、mock_envを使用して環境変数DISCORD_GUILD_IDを設定しました。参考PR

再度お手数ですがご確認をよろしくお願いします🙇‍♂️

goruchanchan

@natsuto6
お疲れ様です！

テスト内でValidTimesChannelクラスのidメソッドが数値としての文字列を返すように修正し、mock_envを使用して環境変数DISCORD_GUILD_IDを設定しました。

詳細に調査、共有くださりありがとうございます！大変勉強になりました🙇‍♂️

１点だけお聞きしたいことを追加しましたので、お手数をおかけしますがご確認よろしくお願いします🙇‍♂️

goruchanchan · 2024-04-07T13:26:39Z

test/models/times_channel_creator_test.rb

@@ -46,6 +48,16 @@ class TimesChannelCreatorTest < ActiveSupport::TestCase
    assert_not user.student_or_trainee?
  end

+  test '#call creates times_url' do


伝え方が悪くて申し訳ないです🙇‍♂️#call のテストに統合してしまってもいいじゃないかなと思った次第です🙇‍♂️

call メソッドは、今回の変更にて time_id に加えて times_url も設定されることになると思うので、その内容を追加するといいのかなと思っています🙇‍♂️

また、#call creates times_url としてテストする場合、ValidTimesChannel によって正しい設定をするだけで終わってしまっているので、当該の正しい設定がされているかの評価があった方がいいと思いました🤔

Takuya-Sakai91 · 2024-04-08T00:28:01Z

@goruchanchan
重ねてのレビューありがとうございます！テストについておっしゃる通りでしたので、修正いたしました。
度々恐れ入りますがご確認をお願いいたします🙇‍♂️

goruchanchan

@natsuto6
ご対応ありがとうございます！！LGTMと思いますので承認いたします！！

また、mock_env の使い方等大変勉強になりました🙇‍♂️今後活用させてもらおうと思います💪ありがとうございました☺️

Takuya-Sakai91 · 2024-04-08T02:48:06Z

@goruchanchan
レビューいただきありがとうございました！！🙇‍♂️
@komagata
メンバーレビューが通りましたので、お手数ですがレビューのほどよろしくお願い致します。

komagata

確認させていただきました。OKです～👌

Takuya-Sakai91 · 2024-04-25T08:44:46Z

@komagata
お疲れ様です。
こちら本番環境にて参加登録いただき、自動で分報チャンネルが作成されることを確認いただくことは可能でしょうか。
また、退会時には分報URLが削除されるかのご確認も併せてご確認いただきたいです。
お手数ですがご確認よろしくお願い致します🙇‍♂️

komagata · 2024-04-25T12:50:37Z

@natsuto6 本番で登録はできないので新しい人が入ったときに確認したいと思います～。

Takuya-Sakai91 · 2024-04-25T14:10:42Z

@komagata
承知いたしました！お手数ですがご対応のほどよろしくお願いいたします。
自分からも都度お声がけさせていただきます。

machida · 2024-04-29T15:14:13Z

@natsuto6 本番で確認できましたー🎉

Takuya-Sakai91 · 2024-04-29T23:18:33Z

@machida
ご確認いただきありがとうございました！😄

Takuya-Sakai91 self-assigned this Mar 20, 2024

Takuya-Sakai91 force-pushed the feature/auto-fill-times-url-on-registration branch 3 times, most recently from f363f6e to f9d5ecb Compare March 26, 2024 12:48

Takuya-Sakai91 marked this pull request as ready for review March 26, 2024 14:55

Takuya-Sakai91 requested a review from goruchanchan March 26, 2024 14:57

komagata force-pushed the main branch from 033db0e to 39dc85f Compare March 27, 2024 07:19

Takuya-Sakai91 force-pushed the feature/auto-fill-times-url-on-registration branch from f9d5ecb to dea9c0c Compare March 30, 2024 23:20

goruchanchan reviewed Mar 31, 2024

View reviewed changes

goruchanchan reviewed Apr 1, 2024

View reviewed changes

Takuya-Sakai91 force-pushed the feature/auto-fill-times-url-on-registration branch from 743e93e to 35d714a Compare April 5, 2024 15:35

goruchanchan reviewed Apr 7, 2024

View reviewed changes

Takuya-Sakai91 added 7 commits April 8, 2024 09:08

分報チャンネル作成後、生成されたチャンネルのURLをユーザーのプロフィールに自動で設定

fe7a026

退会時にユーザーのtimes_urlを削除する機能を実装

f22720a

times_urlを削除する機能の実装に伴い、テストの追記

049a397

ValidTimesChannelのidがnilである問題を解消

7860064

分報チャンネル作成機能のテストを修正

1d01a29

テスト環境での分報チャンネル作成処理を模倣するための修正

379f723

不要なテストケースの削除

c27dc5e

Takuya-Sakai91 force-pushed the feature/auto-fill-times-url-on-registration branch from 35d714a to c27dc5e Compare April 8, 2024 00:09

goruchanchan approved these changes Apr 8, 2024

View reviewed changes

Takuya-Sakai91 requested a review from komagata April 8, 2024 02:46

komagata approved these changes Apr 17, 2024

View reviewed changes

komagata merged commit 891e991 into main Apr 17, 2024
4 checks passed

komagata deleted the feature/auto-fill-times-url-on-registration branch April 17, 2024 07:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

分報チャンネル作成後、生成されたチャンネルのURLをユーザーのプロフィールに自動で設定 #7564

分報チャンネル作成後、生成されたチャンネルのURLをユーザーのプロフィールに自動で設定 #7564

Takuya-Sakai91 commented Mar 20, 2024 •

edited

Loading

Takuya-Sakai91 commented Mar 26, 2024

goruchanchan commented Mar 26, 2024

goruchanchan commented Mar 30, 2024

Takuya-Sakai91 commented Mar 31, 2024

goruchanchan left a comment

goruchanchan Mar 31, 2024

Takuya-Sakai91 Apr 1, 2024

goruchanchan Mar 31, 2024

Takuya-Sakai91 Apr 1, 2024

goruchanchan Mar 31, 2024

Takuya-Sakai91 Apr 1, 2024

goruchanchan Apr 1, 2024

Takuya-Sakai91 commented Apr 1, 2024

goruchanchan left a comment

goruchanchan Apr 1, 2024

goruchanchan Apr 1, 2024

Takuya-Sakai91 Apr 6, 2024

goruchanchan Apr 7, 2024

Takuya-Sakai91 commented Apr 6, 2024

goruchanchan left a comment

goruchanchan Apr 7, 2024

Takuya-Sakai91 commented Apr 8, 2024

goruchanchan left a comment

Takuya-Sakai91 commented Apr 8, 2024

komagata left a comment

Takuya-Sakai91 commented Apr 25, 2024

komagata commented Apr 25, 2024

Takuya-Sakai91 commented Apr 25, 2024

machida commented Apr 29, 2024

Takuya-Sakai91 commented Apr 29, 2024

分報チャンネル作成後、生成されたチャンネルのURLをユーザーのプロフィールに自動で設定 #7564

分報チャンネル作成後、生成されたチャンネルのURLをユーザーのプロフィールに自動で設定 #7564

Conversation

Takuya-Sakai91 commented Mar 20, 2024 • edited Loading

Issue

概要

変更確認方法

Takuya-Sakai91 commented Mar 26, 2024

goruchanchan commented Mar 26, 2024

goruchanchan commented Mar 30, 2024

Takuya-Sakai91 commented Mar 31, 2024

goruchanchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takuya-Sakai91 commented Apr 1, 2024

goruchanchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takuya-Sakai91 commented Apr 6, 2024

goruchanchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takuya-Sakai91 commented Apr 8, 2024

goruchanchan left a comment

Choose a reason for hiding this comment

Takuya-Sakai91 commented Apr 8, 2024

komagata left a comment

Choose a reason for hiding this comment

Takuya-Sakai91 commented Apr 25, 2024

komagata commented Apr 25, 2024

Takuya-Sakai91 commented Apr 25, 2024

machida commented Apr 29, 2024

Takuya-Sakai91 commented Apr 29, 2024

Takuya-Sakai91 commented Mar 20, 2024 •

edited

Loading