Conversation
b2bf8d7 to
02c4df8
Compare
|
""" WalkthroughFAQカテゴリ「給付制度対象講座について」を新設し、該当FAQのデータと表示ロジックをRails Developer Courseページに追加しました。FAQカテゴリ定数の分割、FAQデータ・カテゴリのfixtures追加、FAQ表示用部分テンプレートの新規作成と条件付き表示を実装しています。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WelcomeController
participant View(RailsDevCourse)
participant DB
User->>WelcomeController: rails_developer_courseページにアクセス
WelcomeController->>DB: FAQ_CATEGORY_NAME_FOR_CERTIFIED_RESKILL_COURSESに該当するFAQを取得
WelcomeController->>View(RailsDevCourse): @faqsを渡してテンプレートを描画
alt @faqsが存在
View(RailsDevCourse)->>View(_faq.html.slim): FAQリストを部分テンプレートで表示
else @faqsが空かつ管理者
View(RailsDevCourse)->>View(_faqs_empty.html.slim): 空状態部分テンプレートを表示
end
Possibly related issues
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.75.5)app/controllers/welcome_controller.rbrubocop-minitest extension supports plugin, specify 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ 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
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@e-yanagita-gs 最新のmainを取り込んだので、 git pull --rebase oirign feature/add-faq-to-certified-reskill-course をお願いしますー |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/views/welcome/faqs/_faqs_empty.html.slim (1)
1-20: 実装は適切です。国際化対応の確認をお勧めします。空のFAQ状態の表示ロジックは適切に実装されています。管理者のみに表示されるメッセージと適切なCTAリンクが含まれています。
ただし、ハードコードされた日本語テキストがあります。他の部分でi18n(国際化)対応が行われている場合は、一貫性のために検討してください。
db/fixtures/faqs.yml (2)
228-234: FAQ本文内で制度名を再掲すると読者に親切です質問文だけでなく回答冒頭にも「専門実践教育訓練給付制度」の語を入れると、検索性と可読性が向上します。必須ではありませんがご検討ください。
235-240: 文末表現の統一を推奨他 FAQ では回答末尾を「~です。」で統一していますが、この回答は「ご安心ください。」で終わっています。体言止めや敬体混在が無いよう、文末を揃えると一貫性が高まります。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/controllers/welcome_controller.rb(3 hunks)app/views/welcome/certified_reskill_courses/rails_developer_course/_faq.html.slim(1 hunks)app/views/welcome/certified_reskill_courses/rails_developer_course/index.html.slim(1 hunks)app/views/welcome/faqs/_faqs_empty.html.slim(1 hunks)db/fixtures/faq_categories.yml(1 hunks)db/fixtures/faqs.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tyrrell-IH
PR: fjordllc/bootcamp#8807
File: app/views/welcome/job_support.html.slim:391-392
Timestamp: 2025-06-21T22:30:20.116Z
Learning: このプロジェクトにはRailsの組み込みmarkdownヘルパが利用できないため、markdownのHTML変換には自前でヘルパを実装する必要がある。
app/views/welcome/certified_reskill_courses/rails_developer_course/_faq.html.slim (1)
Learnt from: tyrrell-IH
PR: fjordllc/bootcamp#8807
File: app/views/welcome/job_support.html.slim:391-392
Timestamp: 2025-06-21T22:30:20.116Z
Learning: このプロジェクトにはRailsの組み込みmarkdownヘルパが利用できないため、markdownのHTML変換には自前でヘルパを実装する必要がある。
🔇 Additional comments (5)
db/fixtures/faq_categories.yml (1)
29-32: 新しいFAQカテゴリの追加は適切です。カテゴリ名と位置指定が正しく設定されており、既存のデータ構造と一貫性があります。
app/views/welcome/certified_reskill_courses/rails_developer_course/index.html.slim (1)
51-54: 条件分岐ロジックは適切に実装されています。FAQ表示の条件分岐が正しく実装されており、管理者のみの空状態表示も適切です。ページの最下部への配置も要件通りです。
app/views/welcome/certified_reskill_courses/rails_developer_course/_faq.html.slim (1)
1-23: FAQ表示パーシャルの実装は適切です。セマンティックなHTML構造、適切なループ処理、マークダウン表示の処理が正しく実装されています。
js-markdown-viewクラスの使用により、プロジェクトのカスタムマークダウンレンダリング方式と一貫性があります。app/controllers/welcome_controller.rb (2)
8-9: 定数の分割は適切な設計改善です。FAQ カテゴリの分離により、関心の分離が適切に行われています。定数名も明確で理解しやすいです。
37-37: 法人利用FAQ の取得ロジックが更新されました。新しい定数を使用するように適切に更新されています。
|
@e-yanagita-gs |
|
@e-yanagita-gs @ryufuta 私も同じようにFAQ追加のPR/job_support の下部に就職に関するFAQの一覧を表示する by tyrrell-IH · Pull Request #8807 · fjordllc/bootcampを作成しておりまして、 来週中にはレビューしてお返しできると思います。 |
|
@e-yanagita-gs @tyrrell-IH |
|
@tyrrell-IH @ryufuta |
|
@e-yanagita-gs @ryufuta |
| FAQ_CATEGORY_NAME = '法人利用について' | ||
| FAQ_CATEGORY_NAME_FOR_CORPORATE = '法人利用について' | ||
| FAQ_CATEGORY_NAME_FOR_GRANT_COURSE = '給付制度対象講座について' |
There was a problem hiding this comment.
この定数名のところなんですが、少し相談させていただきたいです🙏
私が作成したPR(/job_support の下部に就職に関するFAQの一覧を表示する)の中で
FAQ_CATEGORY_NAME
を
FAQ_CATEGORY_NAME_FOR_TRAINING に変更しております。参考
このままだとコンフリクトが発生するので、定数名に対する見解を合わせておきたいです。
私がFAQ_CATEGORY_NAME_FOR_TRAININGにした理由は
-
アクション名が
trainingなので、どこのアクションで使うための定数なのか明確になると考えた。 -
別PR(参考)でFAQカテゴリー名「法人利用について」を「企業研修代行について」に変更をした経緯から、現在のFAQカテゴリー名に寄せて定数名を考えるよりアクション名に寄せたほうが変更に強いと考えた(町田さんから、SEO対策で文言を変えたりすることはよくあると聞きました)。
の2点です。
また上記と同様の理由から
FAQ_CATEGORY_NAME_FOR_GRANT_COURSE
も
FAQ_CATEGORY_NAME_FOR_CERTIFIED_RESKILL_COURSESでもいいのかなと思いました。
デメリットとしては単語数が7つと多いですが、今後各アクションのFAQが追加されるごとにFAQ_CATEGORY_NAME_FOR_XXXXの定数が増えていくと思うので、用途のわかりやすさ優先でいいんじゃないかと思いました。
ただ定数名についてはあまり自信を持って指摘できるところではないので、ヤナギタさんのご意見も賜りたいです🙏
There was a problem hiding this comment.
ありがとうございます!
ご提案いただいたとおり、定数名を変更いたしました🙇♀️
おっしゃるようにアクション名に寄せた定数名の方が、コードを読む方にとっても理解しやすいと感じました。
| def logo; end | ||
|
|
||
| def rails_developer_course | ||
| @faqs = FAQCategory.find_by(name: FAQ_CATEGORY_NAME_FOR_GRANT_COURSE).faqs |
There was a problem hiding this comment.
ここについてはCodeRabbitの指摘の通り、nilに対する処理をしたほうが良いのではないかと思います。
例えばFAQ_CATEGORY_NAME_FOR_GRANT_COURSE に
'給付制度対象講座について'ではなく
'給付制度対象講座'というFAQカテゴリー名と一致しない文字列を誤って代入した場合にエラーが発生してしまいます。
また、対応する場合に
FAQCategory.find_by(name: FAQ_CATEGORY_NAME_FOR_GRANT_COURSE)がnilを返した場合に、空の配列を返すのでなくFAQ.noneでActiveRecord_Relationのオブジェクトを返したほうが戻り値を統一できて良いのではと思います。
def rails_developer_course
category = FAQCategory.find_by(name: FAQ_CATEGORY_NAME_FOR_JOB_SUPPORT)
@faqs = category&.faqs || FAQ.none
endThere was a problem hiding this comment.
ありがとうございます!
nilに対する処理を追加しました!
c3e4c4f to
aa2a8e5
Compare
|
@tyrrell-IH |
| layout 'lp' | ||
| DEFAULT_COURSE = 'Railsエンジニア' | ||
| FAQ_CATEGORY_NAME = '企業研修代行について' | ||
| FAQ_CATEGORY_NAME_FOR_TRAINING = '法人利用について' |
There was a problem hiding this comment.
「法人利用」の文言を「企業研修代行」や他の文言に置き換える by tyrrell-IH · Pull Request #8775 · fjordllc/bootcamp
こちらのPRがマージされている影響で、
現在FAQ_CATEGORY_NAME_FOR_TRAININGに入る文字列は’企業研修代行について’が正しいです。
('法人利用について'の状態だと/training以下にFAQが表示されていない状態です)
多分git pull --rebase origin mainした際に変更が紛れ込んだのだと思います。。
修正しておいてください🙏
There was a problem hiding this comment.
失礼しました!
「企業研修代行について」に修正の上、/trainingでFAQが表示されることを確認済です。
fb03584 to
83fa66e
Compare
|
@tyrrell-IH |
|
@tyrrell-IH @komagata |
Issue
概要
「給付金制度対象講座」のページ(
/certified_reskill_courses/rails_developer_course)最下部に、「給付制度対象講座について」カテゴリーのFAQセクションを追加しました。実装に当たっては、「法人利用」のページを参考にしています。
影響範囲
今回のFAQカテゴリ追加に伴い、以下のページでも「給付制度対象講座について」のカテゴリが表示されるようになります。
この挙動が意図通りであるか、念のためご確認をお願いいたします。
/faq)カテゴリ一覧の中に「給付制度対象講座について」が追加されます。
/admin/faq_categories)カテゴリ一覧の中に「給付制度対象講座について」が追加され、ここから編集・削除が可能です。(管理者ユーザーでログイン)
変更確認方法
feature/add-faq-to-certified-reskill-courseをローカルに取り込むgit fetch origin feature/add-faq-to-certified-reskill-coursegit checkout feature/add-faq-to-certified-reskill-coursebin/rails db:seedを実行し、seedデータをデータベースに反映させるforeman start -f Procfile.devでサーバーを立ち上げるScreenshot
変更前
変更後
Summary by CodeRabbit
新機能
データ追加