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

分報チャンネルがアカウントIDの頭文字のグループ内で自動作成されるようにする #6522

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions app/models/discord/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,25 @@ def find_by(id:, token:)
nil
end

def categories(keyword: nil)
channels = Discord::Server.channels(id: guild_id, token: authorize_token)
categories = channels&.select(&:category?)
categories&.select { |category| /#{keyword}/.match? category.name }
end

def channels(id:, token:)
return nil unless enabled?

channels_response = Discordrb::API::Server.channels(token, id)
channels_data = JSON.parse(channels_response.body)

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
Copy link
Contributor

@toke04 toke04 Jun 7, 2023

Choose a reason for hiding this comment

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

[自分用補足]
こちらのrescue の使い方は「rescue修飾子」と言い、
https://docs.ruby-lang.org/ja/latest/doc/spec=2fcontrol.html

このようなコードがあった場合
式1 rescue 式2
式1で例外が発生したら、式2を評価する
スクリーンショット 2023-06-07 19 48 38

log_error(e)
nil
end

def enabled?
guild_id && authorize_token
end
Expand Down
21 changes: 21 additions & 0 deletions app/models/discord/times_category.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# frozen_string_literal: true

module Discord
class TimesCategory
include ActiveSupport::Configurable
config_accessor :default_times_category_id, instance_accessor: false
Copy link
Contributor

@toke04 toke04 Jun 7, 2023

Choose a reason for hiding this comment

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

[自分用補足]
config_accessor は、

  • クラス
  • インスタンス

両方から属性に対して

  • getter
  • setter

の両方の操作が出来る
https://api.rubyonrails.org/classes/ActiveSupport/Configurable/ClassMethods.html

CATEGORY_NAME_KEYWORD = 'ひとりごと・分報'

class << self
def categorize_by_initials(name)
return Discord::TimesCategory.default_times_category_id if name.blank?

times_categories = Discord::Server.categories(keyword: CATEGORY_NAME_KEYWORD)

category_type = /\A[0-9]/.match?(name) ? '数字' : name.upcase[0]
found_times_category = times_categories&.find { |times_category| /#{category_type}/.match? times_category.name }
found_times_category&.id || Discord::TimesCategory.default_times_category_id
end
end
end
end
5 changes: 1 addition & 4 deletions app/models/discord/times_channel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

module Discord
class TimesChannel
include ActiveSupport::Configurable
config_accessor :category_id, instance_accessor: false

class << self
def to_channel_name(username)
username.downcase
Expand All @@ -18,7 +15,7 @@ def initialize(username)
def save
@channel = Discord::Server.create_text_channel(
name: @name,
parent: Discord::TimesChannel.category_id
parent: Discord::TimesCategory.categorize_by_initials(@name)
)

!!@channel
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/discord.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@
bot_token = ENV['DISCORD_BOT_TOKEN'].presence
config.authorize_token = "Bot #{bot_token}" if bot_token
end
Discord::TimesChannel.config.category_id = ENV['DISCORD_TIMES_CHANNEL_CATEGORY_ID'].presence
Discord::TimesCategory.config.default_times_category_id = ENV['DISCORD_TIMES_CHANNEL_CATEGORY_ID'].presence
end
3,586 changes: 3,586 additions & 0 deletions test/cassettes/discord/server/categories.yml

Large diffs are not rendered by default.

3,586 changes: 3,586 additions & 0 deletions test/cassettes/discord/server/categories_with_keyword.yml

Large diffs are not rendered by default.

3,662 changes: 3,662 additions & 0 deletions test/cassettes/discord/server/channels.yml

Large diffs are not rendered by default.

71 changes: 71 additions & 0 deletions test/cassettes/discord/server/channels_with_unauthorized.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 51 additions & 0 deletions test/models/discord/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,5 +109,56 @@ class ServerTest < ActiveSupport::TestCase
Discord::Server.authorize_token = 'skip'
assert_not Discord::Server.enabled?
end

test '.channels' do
logs = []
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
Copy link
Contributor

@toke04 toke04 Jun 7, 2023

Choose a reason for hiding this comment

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

[自分用補足]
こちらでは、actual が配列で取れている事を確認。
下のall?メソッドで一つ一つの確認をしている。

assert actual.all?(Discordrb::Channel)
Copy link
Contributor

@toke04 toke04 Jun 7, 2023

Choose a reason for hiding this comment

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

[自分用補足]
all?メソッドは、引数を渡した場合、パターンに全てマッチしていたら真を返すので、
Discordrb::Channelクラスのインスタンスであるか確認している
https://docs.ruby-lang.org/ja/latest/method/Enumerable/i/all=3f.html

module Discordrb
  class Channel

assert_nil logs.last
end

VCR.use_cassette 'discord/server/channels_with_unauthorized' do
actual = Discord::Server.channels(id: '1234567890123456789', token: 'Bot invalid token')
assert_nil actual
assert_equal '[Discord API] 401: Unauthorized', logs.pop
end

Discord::Server.stub(:enabled?, -> { false }) do
actual = Discord::Server.channels(id: '1234567890123456789', token: 'Bot valid token')
assert_nil actual
Copy link
Contributor

@toke04 toke04 Jun 8, 2023

Choose a reason for hiding this comment

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

[質問]
こちら、

  • サーバーが認証で失敗している(guild_id && authorize_tokenこちらのどれかで失敗している)ので、チャネルの取得が上手くいかない

なので、

  • actualがnilである
  • エラーログも格納されない

という理解でよろしいでしょうか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@syo-tokeshi
はい!おおよそその通りです!

少し補足いたしますと

  • 環境変数に設定しているはずのDiscordサーバー情報である "guild_id" と "authorize_token" のどちらかが nil の場合は『サーバーへのアクセス自体を実行しない』

ので

  • actualがnilである
  • エラーログも格納されない

になります!

Copy link
Contributor

@toke04 toke04 Jun 8, 2023

Choose a reason for hiding this comment

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

ご質問に親切に答えてくださってありがとうございます🙇‍♂️
補足説明助かります🙇‍♂️(理解しました!)

明日の夜までに返信出来る事を目安に頑張ります😄
本日少しバタバタしておりました🙇‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@syo-tokeshi
明日の夜まででなくても全然大丈夫です〜👍️
tokeさんのご都合や体調を優先して下さい!

assert_nil logs.last
end
end
end

test '.categories' do
logs = []
Rails.logger.stub(:error, ->(message) { logs << message }) do
VCR.use_cassette 'discord/server/categories' do
actual = Discord::Server.categories

assert_kind_of Array, actual
assert(actual.all? { |channel| channel.type == Discordrb::Channel::TYPES[:category] })
end

VCR.use_cassette 'discord/server/categories_with_keyword' do
actual = Discord::Server.categories(keyword: 'ひとりごと・分報')

assert_kind_of Array, actual
assert(actual.all? { |channel| channel.type == Discordrb::Channel::TYPES[:category] })
assert(actual.all? { |channel| /ひとりごと・分報/.match? channel.name })
end

Discord::Server.stub(:categories, -> { nil }) do
actual = Discord::Server.categories

assert_nil actual
assert_nil logs.last
end
end
end
end
end
42 changes: 42 additions & 0 deletions test/models/discord/times_category_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true

require 'test_helper'

module Discord
class TimesCategoryTest < ActiveSupport::TestCase
setup do
@default_times_category_id = Discord::TimesCategory.default_times_category_id
Discord::TimesCategory.default_times_category_id = '1111111111111111111'

@stub_times_categories = lambda { |_|
channel_data = { 'id' => '123', 'type' => Discordrb::Channel::TYPES[:category], 'name' => 'A・B・C (ひとりごと・分報)' }
[Discordrb::Channel.new(channel_data, nil)]
}
end

teardown do
Discord::TimesCategory.default_times_category_id = @default_times_category_id
end

test '.categorize_by_initials' do
Discord::Server.stub(:categories, @stub_times_categories) do
actual = Discord::TimesCategory.categorize_by_initials('alice🔰')
assert_equal 123, actual
end
end

test '.categorize_by_initials_with_not_matched_category' do
Discord::Server.stub(:categories, @stub_times_categories) do
actual = Discord::TimesCategory.categorize_by_initials('ありす🔰')
Copy link
Contributor

@toke04 toke04 Jun 8, 2023

Choose a reason for hiding this comment

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

[質問]
すみません、念のため確認なのですが、サインアップ(新規会員登録)時に使用される事は想定しないで、単純に「モデルのメソッドのテスト」という事で追加した、という認識でよろしいでしょうか?
スクリーンショット 2023-06-09 8 50 35

なぜそのような事を言うのかと言いますと、
サインアップ(新規会員登録)からアカウント名を「ありす🔰」では登録出来ないと思うからです。
下に確認のためサインアップ(新規会員登録)のフローを書きます🙇‍♂️

サインアップ(新規会員登録)の処理フロー

前提

  1. Discordへ登録される名前は、login_nameがそのまま登録される。
  2. アカウント登録時は、以下の制限がある。
  • 文字数は3文字以上が必要です。
  • 半角英数字と-(ハイフン)のみが使用できます。
  • 先頭と最後にハイフンを使用することはできません。
  • ハイフンを連続して使用することはできません。
スクリーンショット 2023-06-09 7 59 05

処理の確認

  1. ユーザー作成時に、こちらの処理が走る(新規作成した@userを次に渡す)

    Newspaper.publish(:student_or_trainee_create, @user) if @user.student?

  2. こちらの処理で、受け取った@user(サインアップしたユーザー)を、TimesChannelCreatorクラスのインスタンスとして次に渡す

    Newspaper.subscribe(:student_or_trainee_create, TimesChannelCreator.new)

  3. ユーザーのlogin_nameの値で、ディスコードのチャネルを作成する

    class TimesChannelCreator
    def call(user)
    raise ArgumentError, "#{user.login_name}は現役生または研修生ではありません。" unless user.student_or_trainee?
    times_channel = Discord::TimesChannel.new(user.login_name)
    if times_channel.save
    user.update(times_id: times_channel.id)
    else
    Rails.logger.warn "[Discord API] #{user.login_name}の分報チャンネルが作成できませんでした。"
    end
    end
    end

念のため、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)
      )
スクリーンショット 2023-06-09 8 47 43

Copy link
Contributor

@toke04 toke04 Jun 9, 2023

Choose a reason for hiding this comment

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

長くなりましたが、ようは
Discord::TimesCategory.categorize_by_initials('ありす🔰')

  • このメソッドが、引数としてありす🔰のような値が渡ってきて呼び出されるケースは現状ではないが、メソッドとして、もしこのような値が渡ってきた場合を想定してテストを記述している

という理解でよろしいでしょうか?

[私の考え]
今現在使われるケースを想定したテストを書いた方が良いと思いますので、
こちらにマッチする引数を渡した方が良い気がしました🙇‍♂️
スクリーンショット 2023-06-09 9 02 19
[例] ※日本人&韓国人のユーザーさんがFBCに加入してくる事を想定してこの名前にしました🙇‍♂️(日本人の名前は特に使った方が良いような気がします(大多数なので))

  • sato777
  • kim7777

等です🙇‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

@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つの機能であったりで実施される動作をテストすることが目的となっています。
しかし、実際にシステム開発で作るプログラムでは、画面や機能が複数存在している場合が多いです。
動作の連携に関しては単体テストでは実施しません。
後で紹介する結合テストで行うテストとなります。
スクリーンショット 2023-06-11 10 42 08

Copy link
Contributor Author

Choose a reason for hiding this comment

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

はい!このコードではDiscrodカテゴリーの分類パターンをテストしたいだけなので、ユーザー名に特に意味はないと捉えています!

Copy link
Contributor

Choose a reason for hiding this comment

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

了解です!🙇‍♂️
「単体テスト」の観点から、自分もそのままで良いと思います🙇‍♂️

assert_equal Discord::TimesCategory.default_times_category_id, actual
end
end

test '.categorize_by_initials_with_invalid_name' do
Discord::Server.stub(:categories, @stub_times_categories) do
actual = Discord::TimesCategory.categorize_by_initials(nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらはそのままで良いと思います🙇‍♂️

assert_equal Discord::TimesCategory.default_times_category_id, actual
end
end
end
end
35 changes: 15 additions & 20 deletions test/models/discord/times_channel_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
module Discord
class TimesChannelTest < ActiveSupport::TestCase
setup do
@category_id = Discord::TimesChannel.category_id
Discord::TimesChannel.category_id = nil

@stub_create_text_channel = lambda { |name:, parent:|
Discordrb::Channel.new({
id: '1234567890',
Expand All @@ -17,27 +14,25 @@ class TimesChannelTest < ActiveSupport::TestCase
}
end

teardown do
Discord::TimesChannel.category_id = @category_id
end

test '#save return true' do
Discord::TimesChannel.category_id = '9876543210'
Discord::Server.stub(:create_text_channel, @stub_create_text_channel) do
times_channel = Discord::TimesChannel.new('piyo')

assert_equal true, times_channel.save
assert_equal '1234567890', times_channel.id
assert_equal '9876543210', times_channel.category_id
Discord::TimesCategory.stub(:categorize_by_initials, ->(_) { '9876543210' }) do
Discord::Server.stub(:create_text_channel, @stub_create_text_channel) do
times_channel = Discord::TimesChannel.new('piyo')

assert_equal true, times_channel.save
assert_equal '1234567890', times_channel.id
assert_equal '9876543210', times_channel.category_id
end
end

Discord::TimesChannel.category_id = nil
Discord::Server.stub(:create_text_channel, @stub_create_text_channel) do
times_channel = Discord::TimesChannel.new('piyo')
Discord::TimesCategory.stub(:categorize_by_initials, ->(_) { nil }) do
Discord::Server.stub(:create_text_channel, @stub_create_text_channel) do
times_channel = Discord::TimesChannel.new('piyo')

assert_equal true, times_channel.save
assert_equal '1234567890', times_channel.id
assert_nil times_channel.category_id
assert_equal true, times_channel.save
assert_equal '1234567890', times_channel.id
assert_nil times_channel.category_id
end
end
end

Expand Down