Conversation
Poem
|
masyuko0222
force-pushed
the
feature/feature-toggle
branch
4 times, most recently
from
1ecbe33 to
71851dc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/controllers/concerns/feature_toggle_test.rb (1)
5-47: テストカバレッジが包括的で適切に実装されていますFeatureToggleコンサーンの全ての主要メソッドに対して適切なテストが書かれています。特に以下の点が良く実装されています：
authorized_feature?のboolean判定テストauthorized_feature!の例外処理テストauthorize_featureの複数アクター対応テスト- デフォルト無効状態と有効化後の動作確認
テストをより堅牢にするため、以下の追加テストケースを検討してください：
test 'authorized_feature? with nil current_user' do @controller.current_user = nil assert_not @controller.authorized_feature?(:some_feature) end test 'authorize_feature with empty array' do @controller.authorize_feature(:some_feature, []) assert_not Flipper[:some_feature].enabled?(@controller.current_user) endapp/controllers/concerns/feature_toggle.rb (1)
3-23: フィーチャートグル機能が適切に実装されていますFlipper gemを使用したフィーチャートグル機能が Rails の慣例に従って正しく実装されています。以下の点が特に良く設計されています：
- ActiveSupport::Concernの適切な使用
- ビューでも使用可能なhelper_methodの公開
- 明確な責務分離（チェック、例外処理、有効化）
Array(actors).flattenによる柔軟なアクター指定対応より堅牢性を高めるため、
current_userがnilの場合の明示的な処理を検討してください：def authorized_feature?(feature_key) return false unless current_user Flipper[feature_key].enabled?(current_user) endただし、Flipperはnilアクターを適切に処理するため、現在の実装でも問題ありません。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.traceroute.yml(1 hunks)Gemfile(1 hunks)app/controllers/application_controller.rb(1 hunks)app/controllers/concerns/feature_toggle.rb(1 hunks)config/initializers/flipper.rb(1 hunks)db/migrate/20250616101406_create_flipper_tables.rb(1 hunks)db/schema.rb(1 hunks)test/controllers/concerns/feature_toggle_test.rb(1 hunks)
🔇 Additional comments (6)
Gemfile (1)
35-35: Flipper gemの追加を承認します
flipper-active_recordgemの追加は適切です。バージョン制約も適切に設定されています。ただし、最新の安定版を使用していることを確認することをお勧めします。
flipper-active_record gemの最新バージョンは何ですか？セキュリティアドバイザリはありますか？app/controllers/application_controller.rb (1)
6-6: FeatureToggle concernのincludeは適切ですApplicationControllerへのFeatureToggle concernの追加により、全てのコントローラーでフィーチャートグル機能が利用可能になります。includeの位置も適切です。
.traceroute.yml (1)
38-40: フィーチャートグルメソッドの除外設定が適切ですFeatureToggle concernで提供されるメソッド（
authorize_feature、authorized_feature?、authorized_feature!）をtracerouteの未到達アクション検出から除外する設定は適切です。config/initializers/flipper.rb (1)
1-46: Flipper初期化ファイルの構成が適切です包括的な設定オプションをコメントアウトした形で提供しており、安全で良いアプローチです。メモ化、プレローディング、ストリクトモード、ログ出力などの重要な設定オプションが網羅されています。
デフォルトで何も有効化されていないため、プロジェクトの要件に応じて段階的に設定を有効化できます。
db/migrate/20250616101406_create_flipper_tables.rb (1)
1-22: Flipperテーブル作成マイグレーションは正確な構造です
flipper_featuresとflipper_gatesテーブルの構造は、FlipperのActiveRecordアダプターが期待するスキーマと一致しています。適切なインデックス設定：
flipper_features.keyのユニークインデックスflipper_gatesの複合ユニークインデックス（feature_key,key,value）valueカラムの長さ制限によりインデックスキーサイズエラーを防止
downメソッドでのテーブル削除順序も適切で、可逆的なマイグレーションになっています。db/schema.rb (1)
331-346: Flipperのスキーマ変更が適切に実装されています新しく追加された
flipper_featuresとflipper_gatesテーブルは、Flipper gemの標準的な構造に従っており、適切なインデックスが設定されています。フィーチャーフラグ機能の実装に必要なデータベース基盤が正しく構築されています。
masyuko0222
force-pushed
the
feature/feature-toggle
branch
from
71851dc to
00c8fa6
Compare
Contributor
Author
|
@su-su-su-su |
Contributor
|
@masyuko0222 |
Contributor
|
お疲れ様です。 挙動の確認させていただきました。 としてから に戻して コンソールで確認すると となっています。 |
Contributor
Author
|
@su-su-su-su
こちらの挙動は、Flipper(flipper-active_record gem)の仕様によるものです。
ご指摘の通り、初めて使う方が開発環境でこの挙動に触れると、混乱を招く可能性があると私も感じました。 改めてご確認いただき、ありがとうございました🙇 |
Contributor
|
ご確認ありがとうございます！
なるほどです。Flipperの仕様なのですね。
ありがとうございます。 |
Contributor
Author
|
@su-su-su-su 問題なさそうでしたら、引き続きレビューのほどよろしくお願いいたします🙇 |
Contributor
|
確認させていただきました！ |
su-su-su-su
reviewed
Jul 4, 2025
Contributor
su-su-su-su
left a comment
su-su-su-su
left a comment
There was a problem hiding this comment.
レビュアーの追加ありがとうございます。
確認させていただきました。
私からはApproveとさせていただきます！
su-su-su-su
approved these changes
Jul 4, 2025
・View用(ヘルパー)の`authorized_feature?` ・Controller用の`authorized_feature!`
・ApplicationControllerでincludeしているため、全てのコントローラで、使っているか使っていないかの追跡をしてしまう ・他のconcernsもignoreしているので、それに倣って
masyuko0222
force-pushed
the
feature/feature-toggle
branch
from
00c8fa6 to
376ca0c
Compare
Contributor
Author
|
@su-su-su-su @komagata |
87 tasks
This was referenced Jul 11, 2025
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
概要
ドキュメントとしてまとめています。
上記はリリース後に、BootcampWikiで公開予定です。
変更確認方法
前提：
feature/feature-toggleをローカルに取り込むControllerの制御確認
app/controllers/users_controller.rbのコードを以下のようにするkensyuでログイン/usersにアクセスkimuraでログイン/usersにアクセスViewの制御確認
app/controllers/users_controller.rbのコードを以下のようにするapp/views/application/_global_nav.slimのコードを以下のようにするkensyuでログイン/(ルートページ)にアクセスkimuraでログイン/(ルートページ)にアクセスSummary by CodeRabbit
新機能
ドキュメント
テスト
その他