企業一覧・詳細表示機能をReactからRails viewに移行する #9244
Open
Conversation
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
WalkthroughAPI の companies エンドポイント（コントローラ、Jbuilder、ルート、統合テスト）と複数の React コンポーネントを削除し、Users::CompaniesController#index を拡張してサーバーサイドの Slim 部分テンプレート群へ移行。new-mentor 関連フィクスチャを追加。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Browser
participant Controller as Users::CompaniesController#index
participant CompanyModel as Company
participant UserModel as User
participant Views as Slim Partials
Browser->>Controller: GET /users/companies?target=...
Controller->>CompanyModel: Company.with_attached_logo.order(created_at: :desc)
Controller->>Controller: company_ids = @companies.map(&:id)
Controller->>UserModel: where(company_id: company_ids).users_role(target, allowed_targets).order(:id)
Controller->>Controller: group users by company_id => @company_users
Controller->>Views: render index (companies, company_users, target, allowed_targets)
Views->>Views: render _users_tabs
Views->>Views: render _companies (each) -> _company -> _user_icon (each)
Views-->>Browser: HTML response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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)
app/controllers/users/companies_controller.rb (1)
9-12: QueryObjectパターンの検討を推奨9〜12行目のデータ取得・フィルタリング・グルーピングのロジックがコントローラに直接実装されています。コーディングガイドラインに従い、この複雑なActiveRecordクエリをQueryObjectパターンで抽出することを検討してください（rails-patterns gemのQuery機能を使用）。
理由：
- 複数のチェーンされたクエリ操作を含む
- 将来的に再利用可能性がある
- テスタビリティの向上
- Fat Controllerを避けるため
[As per coding guidelines]
例えば以下のようなQueryObjectを作成できます：
# app/queries/company_users_query.rb class CompanyUsersQuery < Patterns::Query queries User def initialize(company_ids:, target:, allowed_targets:) @company_ids = company_ids @target = target @allowed_targets = allowed_targets end def query relation .with_attached_avatar .where(company_id: @company_ids) .users_role(@target, allowed_targets: @allowed_targets) .order(:id) end endそしてコントローラでは：
def index @target = ALLOWED_TARGETS.include?(params[:target]) ? params[:target] : ALLOWED_TARGETS.first @allowed_targets = ALLOWED_TARGETS @companies = Company.with_attached_logo.order(created_at: :desc) company_ids = @companies.pluck(:id) filtered_users = CompanyUsersQuery.new( company_ids: company_ids, target: @target, allowed_targets: @allowed_targets ).all @company_users = filtered_users.group_by(&:company_id) endapp/views/users/companies/_company.html.slim (1)
12-15: 空のユーザーリストの場合のUI表示を検討してください。
company_usersが空の場合、空の.a-user-icons__items要素のみがレンダリングされます。ユーザーがいない会社の場合、「該当するユーザーがいません」などのメッセージを表示することで、UXが向上する可能性があります。以下のdiffを適用して、空の状態のメッセージを追加できます：
.a-user-icons .a-user-icons__items - - company_users.each do |company_user| - = render 'user_icon', company_user: company_user + - if company_users.any? + - company_users.each do |company_user| + = render 'user_icon', company_user: company_user + - else + .a-user-icons__empty-message + | ユーザーがいません
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
app/controllers/api/users/companies_controller.rb(0 hunks)app/controllers/users/companies_controller.rb(1 hunks)app/javascript/components/Companies.jsx(0 hunks)app/javascript/components/Company.jsx(0 hunks)app/javascript/components/LoadingUserIconPlaceholder.jsx(0 hunks)app/javascript/components/LoadingUsersPageCompaniesPlaceholder.jsx(0 hunks)app/javascript/components/LoadingUsersPageCompanyPlaceholder.jsx(0 hunks)app/views/api/users/companies/_company.json.jbuilder(0 hunks)app/views/api/users/companies/index.json.jbuilder(0 hunks)app/views/users/companies/_companies.html.slim(1 hunks)app/views/users/companies/_company.html.slim(1 hunks)app/views/users/companies/_user_icon.html.slim(1 hunks)app/views/users/companies/_users_tabs.html.slim(1 hunks)app/views/users/companies/index.html.slim(2 hunks)db/fixtures/discord_profiles.yml(1 hunks)db/fixtures/talks.yml(1 hunks)db/fixtures/users.yml(1 hunks)test/integration/api/users/companies_test.rb(0 hunks)
💤 Files with no reviewable changes (9)
- app/controllers/api/users/companies_controller.rb
- app/javascript/components/LoadingUserIconPlaceholder.jsx
- app/javascript/components/Companies.jsx
- app/javascript/components/LoadingUsersPageCompaniesPlaceholder.jsx
- app/views/api/users/companies/index.json.jbuilder
- test/integration/api/users/companies_test.rb
- app/views/api/users/companies/_company.json.jbuilder
- app/javascript/components/LoadingUsersPageCompanyPlaceholder.jsx
- app/javascript/components/Company.jsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
app/controllers/users/companies_controller.rb
🧬 Code graph analysis (1)
app/controllers/users/companies_controller.rb (1)
app/models/user.rb (1)
users_role(510-514)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (9)
db/fixtures/users.yml (1)
1536-1557: LGTM!新しいメンターユーザーのフィクスチャが適切に追加されています。既存のエントリと一貫性があり、必要なフィールドがすべて含まれています。
app/views/users/companies/_user_icon.html.slim (1)
1-3: LGTM!ユーザーアイコンのパーシャルが適切に実装されています。条件付きクラスの適用やデータ属性の設定も正しく行われています。
db/fixtures/talks.yml (1)
235-237: LGTM!新しいメンターユーザーのトークフィクスチャが適切に追加されています。
db/fixtures/discord_profiles.yml (1)
334-337: LGTM!新しいメンターユーザーのDiscordプロファイルフィクスチャが適切に追加されています。
app/views/users/companies/_companies.html.slim (1)
1-6: LGTM!企業一覧のパーシャルが適切に実装されています。ユーザーのいない企業をスキップするロジックも正しく実装されています。
app/views/users/companies/index.html.slim (2)
14-14: LGTM!サーバーサイドレンダリングへの移行が適切に行われています。必要なローカル変数が正しく渡されています。
24-24: LGTM!企業一覧パーシャルへのデータの受け渡しが適切に行われています。
app/views/users/companies/_users_tabs.html.slim (1)
4-8: LGTM!定数からローカル変数への移行が適切に行われています。変数名も明確で、アクティブタブのロジックも正しく更新されています。
app/views/users/companies/_company.html.slim (1)
4-6: フォールバック画像が実装済みです
Company#logo_urlで/images/companies/logos/default.pngへのフォールバックが定義されているため、nilチェックは不要です。
6 tasks
karlley
force-pushed
the
chore/replace-users-companies-to-rails-view
branch
from
8ef72eb to
eed402a
Compare
|
@hirokiej |
|
@karlley さん |
|
@hirokiej |
|
@kutimiti1234 |
viewからcontrollerの定数を直接参照しないように修正 企業への間違ったURLを修正
不要になったルーティングの削除
karlley
force-pushed
the
chore/replace-users-companies-to-rails-view
branch
from
5afca19 to
3e5d475
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/controllers/users/companies_controller.rb (2)
9-12: ページネーションとQueryObjectパターンの検討を推奨します。現在の実装では、すべての企業とユーザーをメモリにロードしています。企業数やユーザー数が増えた場合、パフォーマンスとメモリ使用量の問題が発生する可能性があります。
コーディングガイドラインに従い、以下の改善を検討してください：
- ページネーション: 企業やユーザーの数が多い場合、ページネーション（kaminari等）の導入を検討してください。
- QueryObjectパターン: 11行目の複雑なクエリチェーンは、rails-patterns gemのQueryObjectパターンでカプセル化することで、可読性と再利用性が向上します。
例：
# app/queries/company_users_query.rb class CompanyUsersQuery < Patterns::Query queries User def initialize(company_ids:, target:, allowed_targets:) @company_ids = company_ids @target = target @allowed_targets = allowed_targets end def query relation .with_attached_avatar .where(company_id: @company_ids) .users_role(@target, allowed_targets: @allowed_targets) .order(:id) end endコントローラーでの使用：
def index @target = ALLOWED_TARGETS.include?(params[:target]) ? params[:target] : ALLOWED_TARGETS.first @allowed_targets = ALLOWED_TARGETS @companies = Company.with_attached_logo.order(created_at: :desc) filtered_users = CompanyUsersQuery.new( company_ids: @companies.map(&:id), target: @target, allowed_targets: @allowed_targets ).to_a @company_users = filtered_users.group_by(&:company_id) endBased on coding guidelines
11-11: 長い行を分割することを検討してください。11行目のクエリチェーンが長く、可読性が低下しています。メソッドチェーンを複数行に分割するか、前述のQueryObjectパターンを使用することで改善できます。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/controllers/api/users/companies_controller.rb(0 hunks)app/controllers/users/companies_controller.rb(1 hunks)app/javascript/components/Companies.jsx(0 hunks)app/javascript/components/Company.jsx(0 hunks)app/javascript/components/LoadingUserIconPlaceholder.jsx(0 hunks)app/javascript/components/LoadingUsersPageCompaniesPlaceholder.jsx(0 hunks)app/javascript/components/LoadingUsersPageCompanyPlaceholder.jsx(0 hunks)app/views/api/users/companies/_company.json.jbuilder(0 hunks)app/views/api/users/companies/index.json.jbuilder(0 hunks)app/views/users/companies/_companies.html.slim(1 hunks)app/views/users/companies/_company.html.slim(1 hunks)app/views/users/companies/_user_icon.html.slim(1 hunks)app/views/users/companies/_users_tabs.html.slim(1 hunks)app/views/users/companies/index.html.slim(2 hunks)config/routes/api.rb(0 hunks)db/fixtures/discord_profiles.yml(1 hunks)db/fixtures/talks.yml(1 hunks)db/fixtures/users.yml(1 hunks)test/integration/api/users/companies_test.rb(0 hunks)
💤 Files with no reviewable changes (10)
- app/views/api/users/companies/_company.json.jbuilder
- app/javascript/components/LoadingUserIconPlaceholder.jsx
- app/controllers/api/users/companies_controller.rb
- app/javascript/components/LoadingUsersPageCompanyPlaceholder.jsx
- app/javascript/components/Companies.jsx
- config/routes/api.rb
- app/javascript/components/LoadingUsersPageCompaniesPlaceholder.jsx
- app/views/api/users/companies/index.json.jbuilder
- test/integration/api/users/companies_test.rb
- app/javascript/components/Company.jsx
🚧 Files skipped from review as they are similar to previous changes (6)
- app/views/users/companies/_companies.html.slim
- db/fixtures/discord_profiles.yml
- db/fixtures/users.yml
- app/views/users/companies/_user_icon.html.slim
- app/views/users/companies/_company.html.slim
- app/views/users/companies/index.html.slim
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
app/controllers/users/companies_controller.rb
🧬 Code graph analysis (1)
app/controllers/users/companies_controller.rb (1)
app/models/user.rb (1)
users_role(510-514)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
karlley
force-pushed
the
chore/replace-users-companies-to-rails-view
branch
from
3e5d475 to
a72a42c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/controllers/users/companies_controller.rb (1)
10-12: QueryObjectパターンの使用を検討してください。企業とユーザーのフィルタリングロジックが複数行にわたっており、再利用可能な形にできます。
コーディングガイドラインに従い、この複雑なクエリロジックをQueryObjectパターン（rails-patterns gem）で実装することで、可読性と再利用性が向上します。
コーディングガイドラインに基づく
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/controllers/users/companies_controller.rb(1 hunks)app/views/users/companies/_companies.html.slim(1 hunks)app/views/users/companies/_company.html.slim(1 hunks)app/views/users/companies/_user_icon.html.slim(1 hunks)app/views/users/companies/_users_tabs.html.slim(1 hunks)app/views/users/companies/index.html.slim(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/views/users/companies/_companies.html.slim
- app/views/users/companies/_users_tabs.html.slim
- app/views/users/companies/_company.html.slim
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
app/controllers/users/companies_controller.rb
🧬 Code graph analysis (1)
app/controllers/users/companies_controller.rb (1)
app/models/user.rb (1)
users_role(510-514)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (3)
app/views/users/companies/index.html.slim (1)
14-14: 部分テンプレートへのデータ渡しは適切です。コントローラで準備されたデータを各部分テンプレートに正しく渡しており、Reactからサーバーサイドレンダリングへの移行が適切に実装されています。
Also applies to: 24-24
app/controllers/users/companies_controller.rb (1)
9-9: 企業数が多い場合のパフォーマンスを確認してください。全ての企業をページネーションなしで読み込んでいます。企業数が多い場合、パフォーマンスの問題が発生する可能性があります。
想定される企業数を確認し、必要に応じてページネーションの実装を検討してください。
app/views/users/companies/_user_icon.html.slim (1)
1-3: company_userのメソッドはUserDecoratorおよびモデルに定義済みです。追加対応不要です
|
@karlley |
|
@kutimiti1234 |
|
@karlley ただ1点、PRのタイトルはbranch名以外をきちんと設定したほうが良いかなと思いました。 |
kutimiti1234
approved these changes
Oct 18, 2025
karlley
changed the title
|
@kutimiti1234 |
|
@komagata |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
概要
企業一覧・詳細表示機能をReactからRails viewに移行
変更確認方法
chore/replace-users-companies-to-rails-viewをローカルに取り込む各項目ごとに以下を確認
表示とリンク
komagataでログインロールでのフィルタリング
komagataでログイン.is-activeが付与され、選択色になっているレスポンシブ対応
komagataでログイン新規ユーザー用アイコンの追加デザイン
komagataでログインニューメンターユーザーのアイコンで以下を確認.is-new-userクラスが付与され、新規ユーザー用デザインが追加されているScreenshot
見た目の変更はありません
Summary by CodeRabbit
新機能
リファクタ
チョア
テスト
Closes #9047