Conversation
ymmtd0x0b
force-pushed
the
feature/change-the-category-of-the-discord-channel-that-is-automatically-created-when-you-join
branch
from
8f74196 to
4d7a38f
Compare
|
@syo-tokeshi |
|
@ymmtd0x0b よろしくお願いいたしますー😊 早ければ土曜日までにレビューします🙇♂️(なかなかタフそうですね、、一緒に勉強させて下さい！！) |
|
@ymmtd0x0b 上の写真の botもディスコードに参加しております 
「ピコっ」という音が鳴り、反応している気はするのですが、何か解決するヒント等、持ち合わせてないでしょうか？ |
|
名前はBootcamp2としてますが、設定自体は上手くいってると思うんですよね、、 
|
|
@syo-tokeshi |
|
あ、ちょっと解決しそうです🙇♂️ カテゴリーを作成からグループを作るのですね カテゴリーを作成からグループを作り直しました🙇♂️ |
|
@ymmtd0x0b 解決しました |
|
@syo-tokeshi [追記] |
|
@ymmtd0x0b これで安心してコードレビューに取り掛かれそうです😆(明日以降になります🙇♂️) 夜遅くにすみませんでした！🙇♂️
かしこまりました！！🙇♂️ |
|
@syo-tokeshi |
|
|
||
| bot = Discordrb::Bot.new(token: token, log_mode: :silent) | ||
| channels_data.map { |channel_data| Discordrb::Channel.new(channel_data, bot) } | ||
| rescue Discordrb::Errors::CodeError => e |
There was a problem hiding this comment.
[自分用補足]
こちらのrescue の使い方は「rescue修飾子」と言い、
https://docs.ruby-lang.org/ja/latest/doc/spec=2fcontrol.html
このようなコードがあった場合
式1 rescue 式2
式１で例外が発生したら、式２を評価する
| module Discord | ||
| class TimesCategory | ||
| include ActiveSupport::Configurable | ||
| config_accessor :default_times_category_id, instance_accessor: false |
There was a problem hiding this comment.
[自分用補足]
config_accessor は、
- クラス
- インスタンス
両方から属性に対して
- getter
- setter
の両方の操作が出来る
https://api.rubyonrails.org/classes/ActiveSupport/Configurable/ClassMethods.html
| VCR.use_cassette 'discord/server/channels' do | ||
| actual = Discord::Server.channels(id: '1234567890123456789', token: 'Bot valid token') | ||
| assert_kind_of Array, actual | ||
| assert actual.all?(Discordrb::Channel) |
There was a problem hiding this comment.
[自分用補足]
all?メソッドは、引数を渡した場合、パターンに全てマッチしていたら真を返すので、
Discordrb::Channelクラスのインスタンスであるか確認している
https://docs.ruby-lang.org/ja/latest/method/Enumerable/i/all=3f.html
module Discordrb
class Channel
| Rails.logger.stub(:error, ->(message) { logs << message }) do | ||
| VCR.use_cassette 'discord/server/channels' do | ||
| actual = Discord::Server.channels(id: '1234567890123456789', token: 'Bot valid token') | ||
| assert_kind_of Array, actual |
There was a problem hiding this comment.
[自分用補足]
こちらでは、actual が配列で取れている事を確認。
下のall?メソッドで一つ一つの確認をしている。
|
|
||
| Discord::Server.stub(:enabled?, -> { false }) do | ||
| actual = Discord::Server.channels(id: '1234567890123456789', token: 'Bot valid token') | ||
| assert_nil actual |
There was a problem hiding this comment.
[質問]
こちら、
- サーバーが認証で失敗している(
guild_id && authorize_tokenこちらのどれかで失敗している)ので、チャネルの取得が上手くいかない
なので、
actualがnilである- エラーログも格納されない
という理解でよろしいでしょうか？
There was a problem hiding this comment.
@syo-tokeshi
はい！おおよそその通りです！
少し補足いたしますと
- 環境変数に設定しているはずのDiscordサーバー情報である "guild_id" と "authorize_token" のどちらかが
nilの場合は『サーバーへのアクセス自体を実行しない』
ので
- actualがnilである
- エラーログも格納されない
になります！
There was a problem hiding this comment.
ご質問に親切に答えてくださってありがとうございます🙇♂️
補足説明助かります🙇♂️(理解しました！)
明日の夜までに返信出来る事を目安に頑張ります😄
本日少しバタバタしておりました🙇♂️
There was a problem hiding this comment.
@syo-tokeshi
明日の夜まででなくても全然大丈夫です〜👍️
tokeさんのご都合や体調を優先して下さい！
|
|
||
| test '.categorize_by_initials_with_not_matched_category' do | ||
| Discord::Server.stub(:categories, @stub_times_categories) do | ||
| actual = Discord::TimesCategory.categorize_by_initials('ありす🔰') |
There was a problem hiding this comment.
[質問]
すみません、念のため確認なのですが、サインアップ（新規会員登録）時に使用される事は想定しないで、単純に「モデルのメソッドのテスト」という事で追加した、という認識でよろしいでしょうか？
なぜそのような事を言うのかと言いますと、
サインアップ（新規会員登録）からアカウント名を「ありす🔰」では登録出来ないと思うからです。
下に確認のためサインアップ（新規会員登録）のフローを書きます🙇♂️
サインアップ（新規会員登録）の処理フロー
前提
- Discordへ登録される名前は、
login_nameがそのまま登録される。 - アカウント登録時は、以下の制限がある。
- 文字数は3文字以上が必要です。
- 半角英数字と-（ハイフン）のみが使用できます。
- 先頭と最後にハイフンを使用することはできません。
- ハイフンを連続して使用することはできません。
処理の確認
-
ユーザー作成時に、こちらの処理が走る(新規作成した@userを次に渡す)
bootcamp/app/controllers/users_controller.rb
Line 127 in 5a79762
-
こちらの処理で、受け取った@user(サインアップしたユーザー)を、
TimesChannelCreatorクラスのインスタンスとして次に渡す
bootcamp/config/initializers/newspaper.rb
Line 31 in 5a79762
-
ユーザーの
login_nameの値で、ディスコードのチャネルを作成する
bootcamp/app/models/times_channel_creator.rb
Lines 3 to 14 in 5a79762
念のため、saveメソッドまでここに書いておきます(ここで、login_nameをそのままディスコードのチャネル名に使っている感じですよね🙇♂️)
module Discord
class TimesChannel
class << self
def to_channel_name(username)
username.downcase
end
end
def initialize(username)
@name = Discord::TimesChannel.to_channel_name(username)
end
def save
@channel = Discord::Server.create_text_channel(
name: @name,
parent: Discord::TimesCategory.categorize_by_initials(@name)
)
There was a problem hiding this comment.
長くなりましたが、ようは
Discord::TimesCategory.categorize_by_initials('ありす🔰')
- このメソッドが、引数として
ありす🔰のような値が渡ってきて呼び出されるケースは現状ではないが、メソッドとして、もしこのような値が渡ってきた場合を想定してテストを記述している
という理解でよろしいでしょうか？
[私の考え]
今現在使われるケースを想定したテストを書いた方が良いと思いますので、
こちらにマッチする引数を渡した方が良い気がしました🙇♂️
[例] ※日本人&韓国人のユーザーさんがFBCに加入してくる事を想定してこの名前にしました🙇♂️(日本人の名前は特に使った方が良いような気がします(大多数なので))
- sato777
- kim7777
等です🙇♂️
There was a problem hiding this comment.
@ymmtd0x0b
お疲れ様です。
後から考えてみたのですが、
- モデルのテスト = 単体テスト
とは、他のクラスの事は考慮せず、それ単体(今回で言うとDiscord::TimesCategoryクラス)の機能をテストする、という観点で言うと、ヤマモトさんの今のままのテストコードで良い気がしてきました🙇♂️
https://and-engineer.com/articles/X_-EBRAAACcAJnau
つまり、私が最初に言ってた「usersコントローラーから呼ばれてますよね」みたいな理屈は関係なく、
あくまでDiscord::TimesCategoryクラスのメソッドをテストすれば良いと思うので、このままで良さそうです(今のテストで十分、メソッドのテストが出来ていると思います)🙇♂️
参考文献
https://and-engineer.com/articles/X_-EBRAAACcAJnau
単体テストは、UT工程とも呼ばれ、「Unit Test」の通り、メソッドや関数などの1つ1つの小さな単位ごとに行うテストのことです。
1つの画面であったり、1つの機能であったりで実施される動作をテストすることが目的となっています。
しかし、実際にシステム開発で作るプログラムでは、画面や機能が複数存在している場合が多いです。
動作の連携に関しては単体テストでは実施しません。
後で紹介する結合テストで行うテストとなります。
There was a problem hiding this comment.
はい！このコードではDiscrodカテゴリーの分類パターンをテストしたいだけなので、ユーザー名に特に意味はないと捉えています！
There was a problem hiding this comment.
了解です！🙇♂️
「単体テスト」の観点から、自分もそのままで良いと思います🙇♂️
|
|
||
| test '.categorize_by_initials_with_invalid_name' do | ||
| Discord::Server.stub(:categories, @stub_times_categories) do | ||
| actual = Discord::TimesCategory.categorize_by_initials(nil) |
|
@ymmtd0x0b 私自身もの凄く学ばせて頂きました😄 クラス設計とかもめちゃくちゃ良いと思いました😊 私が今回話し合いたいのはこちら２点だけになります
実装自体も、本番にマージしても問題は起きないと思いました🙇♂️ [私の検証証跡] |
|
@ymmtd0x0b 結論から言うと、「今回のヤマモトさんのコードに修正が必要な箇所はなさそう」というのが私の結論です🙇♂️ 最初は「変更が必要そう」と発言して、失礼しました🙇♂️ |
toke04
left a comment
toke04
left a comment
There was a problem hiding this comment.
変更して欲しい箇所がございませんので、Approveさせて頂きます🙇♂️
勉強させていただきました！！😄
|
@syo-tokeshi テスト名に使用するユーザー名は現状の仕様に合わせることも可能ですが、Approve して頂いた後に変更すると少しややこしくなりそうなので、今回はこのまま行こうと思います！ |
@ymmtd0x0b
等、めっちゃ勉強になりました！
はい！それで良いと思います😆 |
ymmtd0x0b
force-pushed
the
feature/change-the-category-of-the-discord-channel-that-is-automatically-created-when-you-join
branch
from
4d7a38f to
9623033
Compare
|
@komagata |
komagata
deleted the
feature/change-the-category-of-the-discord-channel-that-is-automatically-created-when-you-join
branch
3 participants
Issue
概要
入会時に自動作成される分報チャンネルのグループを『 固定の分報グループ (
新規🎉(ひとりごと・分報)) 』から『 アカウントIDの頭文字に合致する分報グループ (数字・A・B・C (ひとりごと・分報)等 ) 』に変更しました変更確認方法
※変更を確認するためには、事前に確認用のDiscordサーバーを作成し
DiscordサーバーのID/デフォルトグループのID/認証用のBotトークンを環境変数に設定する必要があります事前準備
準備手順
Discordサーバーの作成
#5454 の『Discordへの通知を確認するための準備』の手順に従い確認用のDiscordサーバーを作成して下さい ( ※WebhookURLの取得は不要です )
DiscordサーバーのIDを取得する
#6185 の『Discord サーバーのIDを取得する』の手順に従いDiscordサーバーのIDを控えて下さい
デフォルトグループのIDを取得する
3-1. 確認用のDiscordサーバーでデフォルトグループを作成します ( 例：
新規🎉(ひとりごと・分報))3-2. #6185 の『分報チャンネルのカテゴリーのIDを取得する』の手順に従ってDiscordサーバーのIDを控えて下さい
認証用のBotトークンを追加する
#6185 の『Discord Bot のトークン情報を取得する』の手順に従い、Botの作成・確認用のDiscordサーバーへ追加を行った後にBotトークンを控えて下さい
環境変数に
DiscordサーバーのID/デフォルトグループのID/認証用のBotトークンを設定する手順2~3で控えた情報を#6185 の『 注意・伝達事項 』に記載されている手順に従い環境変数へ設定して下さい
準備後の状態
ここまでの作業の結果をご参考までに例示します
確認手順
数字・A・B・C (ひとりごと・分報)の名前でグループ (カテゴリー)を作成するfeature/change-the-category-of-the-discord-channel-that-is-automatically-created-when-you-joinをローカルに取り込むbin/rails sでサーバーを立ち上げるalice) として参加登録を行うzoe) として参加登録を行う数字・A・B・C (ひとりごと・分報)グループにalice、新規🎉(ひとりごと・分報)にzoeの分報チャンネルがそれぞれ作成されている事を確認するScreenshot
変更前
変更後
補足情報
以下に動作確認やレビューに役立つと思われる情報を記載します
Discord Web API におけるチャンネルについて
以下に挙げた機能は全て Discord Web API 上で『チャンネル』として扱われます
※このPRにおけるグループとカテゴリは同義です
アカウントIDに使用可能な文字と使用ルール
以下に示すルールが現時点におけるアカウントIDに使用可能な文字になります
グループ分類の規則
アカウントIDの頭文字によって分類するグループの規則は以下のようになります
M (ひとりごと・分報)
※１
将来的な仕様変更が発生した場合でも想定内の問題として対応・異常の発見ができるように、アカウントIDの頭文字が半角英数字以外でも『デフォルトグループ』に分類されるよう対策しています ( 質問・雑談チャンネルにて了承を頂きました )
※２
現在の仕様ではFBCのDiscordサーバーにある
新規🎉(ひとりごと・分報)のチャンネルIDを環境変数経由で取得し、新規ユーザーの分報チャンネルのグループとして設定されています。このPRのマージ後はチャンネル名を変更し、適切な分類ができない場合に対処するためのグループとして再利用します ( 質問・雑談チャンネルにて了承を頂きました )また、アカウントIDの頭文字が半角英数字だったとしても適切なグループに分類できない場合は『デフォルトグループ』へ分類されるように設定しています
例えば、何らかの要因で
Aの文字を含むグループが存在しない場合、アカウントIDの頭文字がaから始まるユーザーの分報チャンネルはデフォルトグループに作成されますその他
もし、より詳しい情報が必要な場合は以下のリンクが参考になると思いますので、参照して頂ければと思います