Conversation
WalkthroughWelcomeControllerに就職用カテゴリ定数と共通ヘルパー Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant RailsApp as WelcomeController
participant DB as FAQCategory/FAQ
Browser->>RailsApp: GET /welcome/job_support
RailsApp->>DB: FAQCategory.find_by(name: FAQ_CATEGORY_NAME_FOR_JOB_SUPPORT)
alt category found
DB-->>RailsApp: category with faqs
RailsApp->>RailsApp: @faqs = category.faqs.order(:position)
else not found
DB-->>RailsApp: nil
RailsApp->>RailsApp: @faqs = FAQ.none
end
RailsApp-->>Browser: render job_support
alt @faqs.present?
RailsApp-->>Browser: render _faqs partial (faqs: @faqs)
else admin user
RailsApp-->>Browser: render welcome/faqs/faqs_empty partial
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 RuboCop (1.76.1)test/system/faqs/job_support_faqs_test.rbrubocop-minitest extension supports plugin, specify app/controllers/welcome_controller.rbrubocop-minitest extension supports plugin, specify Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/controllers/welcome_controller.rb (1)
38-40:trainingアクションでも同様の nil ガードを追加すべきです上記
job_supportと同じロジックなので、同じ不具合が発生します。重複コードを避ける意味でも共通メソッド化を検討してください。
🧹 Nitpick comments (2)
app/controllers/welcome_controller.rb (1)
8-9: 定数は.freezeしておくと誤代入を防げます- FAQ_CATEGORY_NAME_FOR_TRAINING = '法人利用について' - FAQ_CATEGORY_NAME_FOR_JOB_SUPPORT = '就職について' + FAQ_CATEGORY_NAME_FOR_TRAINING = '法人利用について'.freeze + FAQ_CATEGORY_NAME_FOR_JOB_SUPPORT = '就職について'.freezeapp/views/welcome/job_support.html.slim (1)
384-394:@faqsが空・nil の場合の表示崩れ対策コントローラ側で nil ガードしても、FAQ が 0 件の場合はループが実行されず空セクションになります。以下のように分岐を追加すると UX が向上します。
- if @faqs.present? - @faqs.each do |faq| ... - else p.lp-faq__empty | 現在、就職に関する FAQ はありません。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/controllers/welcome_controller.rb(3 hunks)app/views/welcome/job_support.html.slim(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
tyrrell-IH
force-pushed
the
feature/display_FAQ_bottom_of_/job_support
branch
3 times, most recently
from
3d6b2b4 to
4c1f769
Compare
|
@machida
|
|
@tyrrell-IH 了解ですー！連絡ありがとうございます |
|
@machida |
|
@tyrrell-IH ありがとうございます！今日、こちらいじっておきますー |
machida
force-pushed
the
feature/display_FAQ_bottom_of_/job_support
branch
2 times, most recently
from
a14b6f3 to
ca4a4f5
Compare
|
@tyrrell-IH お待たせしてすいません。最新のmainを取り込んだ上で修正を追加しました。レビューに進めてください🙏 |
|
テストも通りました👍 |
|
@karlley 2週間ほどであればレビューの期限を設けていただいて大丈夫です👌 |
|
@tyrrell-IH
承知しました！ |
karlley
left a comment
karlley
left a comment
There was a problem hiding this comment.
@tyrrell-IH
レビュー大変お待たせしました🙇♂️
修正内容について問題無さそうです！
パーシャルへの切り出しや命名変更も行われており、とても良いアプローチだなと感じました👍
1点だけ指摘では無いのですがwelcome_controller#job_support に対するsystemテストが存在していないことが気になりました。
ただ、以下の理由かこちらのテストは必須ではないと考えていますので対応はお任せします。
- controllerに対するテストは必須では無いとの記載有り(https://bootcamp.fjord.jp/reports/3300)
welcome_controller#job_supportは既存のメソッドなので当PRでテストを追加するべきではなさそう(テストが必要であれば#job_supportが追加された時点でテストが追加されているはず)
確認よろしくお願いします！
|
@karlley ご指摘いただきありがとうございました！ テスト作成後再度連絡します〜 |
tyrrell-IH
force-pushed
the
feature/display_FAQ_bottom_of_/job_support
branch
from
ca4a4f5 to
a6c0ec3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
test/fixtures/faqs.yml (1)
11-170: faq_categories.yml に未定義のラベルがありますtest/fixtures/faqs.yml で参照している
faq_category24が、test/fixtures/faq_categories.ymlに定義されていません。テストで関連解決に失敗します。対応：
- test/fixtures/faq_categories.yml に以下のような定義を追加してください
例）faq_category24: name: 匿名参加 description: 匿名で参加できることの説明文
- または、参照先のラベルを既存の番号に修正してください
♻️ Duplicate comments (1)
app/controllers/welcome_controller.rb (1)
21-24: nilガードが追加されNoMethodErrorを回避できています過去指摘の nil ガードが反映されており、例外を防げています。挙動は
training,rails_developer_courseと同様で一貫性も取れています。
🧹 Nitpick comments (6)
app/views/welcome/certified_reskill_courses/rails_developer_course/index.html.slim (1)
51-54: FAQ表示の共通化（ViewComponent化）の検討（任意）本PRで複数ページが同仕様のFAQセクションを持つようになっているため、将来的には ViewComponent（例: FaqSectionComponent）に寄せると見た目・文言・構造の変更を1箇所で管理でき、保守性が上がります。今すぐの必須ではありませんが、継続的な拡張が見込まれる場合は検討の価値があります。
app/controllers/welcome_controller.rb (2)
10-10: カテゴリ名の定数化は良い判断ビュー/テストと同一のカテゴリ名を定数で集約できており、変更耐性が上がっています。将来的に名称変更の可能性がある場合は、DB側にスラッグを持たせてスラッグで参照する設計も選択肢です（名称変更に強くなります）。
21-24: 重複ロジックのDRY化（任意）
job_support,training,rails_developer_courseで同型の代入処理が繰り返されています。プライベートヘルパーに寄せると保守性が上がります。適用例（この範囲の差分 + 追補コード）:
def job_support - category = FAQCategory.find_by(name: FAQ_CATEGORY_NAME_FOR_JOB_SUPPORT) - @faqs = category&.faqs || FAQ.none + @faqs = faqs_for(FAQ_CATEGORY_NAME_FOR_JOB_SUPPORT) endこの変更を支えるプライベートメソッド（該当箇所以外の追加）:
private def faqs_for(category_name) FAQCategory.find_by(name: category_name)&.faqs || FAQ.none endtest/system/faqs/job_support_faqs_test.rb (3)
6-6: テスト側のカテゴリ名の重複定義を避けるコントローラの定数を参照すれば、名称変更時のテスト破綻を防げます。
- FAQ_CATEGORY_NAME = '就職について' + FAQ_CATEGORY_NAME = WelcomeController::FAQ_CATEGORY_NAME_FOR_JOB_SUPPORT
11-11: 見出しテキストのマッチ方法を空白正規化に強い形へ改行や空白の差異で不安定になり得るため、正規表現マッチにする方が堅牢です。
- assert_selector 'h3.lp-content-title', text: "#{FAQ_CATEGORY_NAME}\nよくある質問" + assert_selector 'h3.lp-content-title', text: /#{Regexp.escape(FAQ_CATEGORY_NAME)}\s*よくある質問/
27-27: CTA検証をリンク要素＋hrefで厳密にボタンの見た目クラスではなく、リンクのテキストと遷移先を検証すると誤検知を避けられます。
- assert_selector '.a-button', text: 'よくある質問を追加する' + assert_link 'よくある質問を追加する', href: '/admin/faqs/new'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
app/controllers/welcome_controller.rb(2 hunks)app/views/welcome/certified_reskill_courses/rails_developer_course/index.html.slim(1 hunks)app/views/welcome/job_support.html.slim(1 hunks)app/views/welcome/job_support/_faqs.html.slim(1 hunks)app/views/welcome/training.html.slim(1 hunks)app/views/welcome/training/_faqs.html.slim(1 hunks)test/fixtures/faqs.yml(11 hunks)test/system/faqs/job_support_faqs_test.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/views/welcome/job_support/_faqs.html.slim
- app/views/welcome/training/_faqs.html.slim
- app/views/welcome/training.html.slim
- app/views/welcome/job_support.html.slim
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
test/system/faqs/job_support_faqs_test.rbapp/controllers/welcome_controller.rb
test/**/*
⚙️ CodeRabbit Configuration File
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/faqs/job_support_faqs_test.rbtest/fixtures/faqs.yml
⏰ 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 (2)
test/fixtures/faqs.yml (1)
11-170: fixtures内の関連名の統一はOK
faq_category: faq_category*への統一で、関連（belongs_to :faq_category）の表現として整合しています。この変更自体は妥当です。app/views/welcome/certified_reskill_courses/rails_developer_course/index.html.slim (1)
51-54: FAQ部分テンプレートの呼び出し先変更は妥当（命名の一貫性向上）
faq->faqsへの置き換えとfaqs: @faqsのローカル渡しは、他ページと統一されていて分かりやすいです。
tyrrell-IH
force-pushed
the
feature/display_FAQ_bottom_of_/job_support
branch
from
fd75f04 to
9b6867d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/controllers/welcome_controller.rb (1)
79-81: faqs_for の実装は適切。faqアクションにも適用して NoMethodError の可能性を潰しましょう
FAQCategory.find_by(name: params[:category]).faqsを使っているfaqアクション（Line 32-37 付近）は、無効なcategoryパラメータで 500 になり得ます。ここもfaqs_forを使って統一すると安全です（以前の指摘と同趣旨）。以下の変更をご検討ください（選択行以外の変更提案のため参考diffです）:
--- a/app/controllers/welcome_controller.rb +++ b/app/controllers/welcome_controller.rb @@ if params[:category].present? - faq_category = FAQCategory.find_by(name: params[:category]) - @faqs = faq_category.faqs + @faqs = faqs_for(params[:category]) else @faqs = FAQ.order(:position) end残存パターンの洗い出し用スクリプトです（レポジトリルートで実行）。
find_by(name: ...).faqsの直呼びが他にないか確認できます。#!/bin/bash set -euo pipefail echo "Scanning for potential nil-unsafe usages of find_by(name: ...).faqs ..." rg -nPU -C2 --type rb '(?s)FAQCategory\.find_by\s*\(\s*name:\s*.*?\)\s*\.\s*faqs\b'
🧹 Nitpick comments (1)
app/controllers/welcome_controller.rb (1)
10-10: FAQカテゴリ名の定数追加はOK。ただし「名前」依存は脆いので将来的に slug 等への切り替えを検討を
DB上のカテゴリ名に依存すると、名称変更（管理画面から編集可能な場合など）で表示が壊れます。恒久的キー（slug/key カラム）で参照する設計へ移行できると堅牢です。現状はfaqs_forがFAQ.noneで安全にフォールバックするので、緊急度は低です。カテゴリ名は運用上変更されない前提でしょうか？変更されうる場合は、将来的に
find_by(slug: ...)へ移行する方針のご検討をお願いします。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/controllers/welcome_controller.rb(5 hunks)test/system/faqs/job_support_faqs_test.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/system/faqs/job_support_faqs_test.rb
🧰 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/welcome_controller.rb
⏰ 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/controllers/welcome_controller.rb (3)
21-23: job_support: 共通ヘルパーでの取得に統一できていて良い（nilセーフ、並び順もOK）
重複排除できており、カテゴリ未存在時もFAQ.noneで安全に扱えるため、ビュー側の3パターン分岐にフィットします。
41-41: training: FAQ取得の共通化は適切
従来のfind_by(...).faqs直呼びからの移行で例外要因を低減できています。
61-61: rails_developer_course: 共通ヘルパー適用は妥当
カテゴリ未存在時のフォールバックも含め、挙動が統一されていて良いです。
tyrrell-IH
force-pushed
the
feature/display_FAQ_bottom_of_/job_support
branch
from
9b6867d to
5091f36
Compare
|
@karlley テストに関する修正参考のPR
codeRabbitの指摘commit: 342cab7 |
karlley
left a comment
karlley
left a comment
There was a problem hiding this comment.
@tyrrell-IH
大変お待たせしました🙇♂️
テストの追加対応ありがとうございました。
ディレクトリの整理やリファクタリングもされており、とても見やすかったです！
問題無さそうなのでApproveさせていただきました😄
tyrrell-IH
force-pushed
the
feature/display_FAQ_bottom_of_/job_support
branch
from
5091f36 to
774703c
Compare
tyrrell-IH
force-pushed
the
feature/display_FAQ_bottom_of_/job_support
branch
from
774703c to
1d590ca
Compare
|
@komagata |
|
以下の画像 できれば
以下の画像 |
|
本番で確認できました 🎉 |
4 participants
Issue
概要
bootcamp.fjord.jp/job_supportのページの下部に「就職についてよくある質問」の項目と、それに関連するFAQが表示されるようにしました。仕様についての詳細はissueの方をご確認ください。
変更確認方法
feature/display_FAQ_bottom_of_/job_supportをローカルに取り込み、当該ブランチに切り替える。表示方法が3パターンあります。
FAQが1件以上ある場合
FAQが0件かつadmin権限で/job_supporへアクセスしている場合
FAQが0件かつ非admin権限で/job_supporへアクセスしている場合
最下方へスクロールし、スクリーンショット(変更後)の「FAQが0件かつ非admin権限で/job_supporへアクセスしている場合」の画像を参考に、以下の内容を確認
Screenshot
変更前
変更後
FAQが1件以上ある場合
FAQが0件かつadmin権限で/job_supporへアクセスしている場合
FAQが0件かつ非admin権限で/job_supporへアクセスしている場合
Summary by CodeRabbit
新機能
テスト
雑務