Conversation
📝 WalkthroughWalkthroughAPI とモデルに給付金コース専用スコープを追加し、フロントで給付金フィルタ UI を動的に読み込んでクエリに Changes
Sequence Diagram(s)sequenceDiagram
participant User as ユーザー
participant React as React(Reports)
participant GrantFilter as GrantFilter(JS)
participant API as API::ReportsController
participant DB as データベース
User->>React: ページ表示（`practiceSourceId` を受け取る）
React->>React: isGrantCourse を判定、withGrant を初期化
React->>GrantFilter: useEffect で grant-filter.js を動的ロード
GrantFilter->>GrantFilter: render(element) でタブ描画・イベント登録
User->>GrantFilter: 「給付金コース」タブをクリック
GrantFilter->>React: setWithGrant(true)
React->>API: GET /api/reports?with_grant=true（他パラメータ含む）
API->>DB: with_grant_practices スコープを適用して問い合わせ
DB-->>API: フィルタ済みレポートを返却
API-->>React: JSON レスポンス
React->>React: レポート一覧を更新して表示
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
karlley
changed the title
karlley
changed the title
![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
🤖 Fix all issues with AI agents
In `@app/javascript/grant-filter.js`:
- Around line 9-21: The template string in render(element) contains a malformed
closing tag `</nav` (missing the trailing '>') which breaks HTML parsing; update
the template literal in the render(element) method (in grant-filter.js) to use a
proper closing `</nav>` tag so the generated markup is valid.
In `@db/fixtures/users.yml`:
- Around line 1559-1577: The name_kana field is set in kanji instead of
katakana; update the fixture entry for login_name grant-course by replacing
name_kana: 給付金コースのユーザー with the katakana form (e.g. name_kana: キュウフキンコースノユーザー)
so it matches other user fixtures and maintains consistency with name:
給付金コースのユーザー.
🧹 Nitpick comments (3)
.rubocop.yml (1)
31-34: 複雑度の除外設定について検討が必要です。
Metrics/PerceivedComplexityの除外は、コントローラーの複雑度が増したことへの対応として理解できます。ただし、RuboCop の警告を除外するよりも、コントローラーのロジックをリファクタリングする方が望ましい場合があります。例えば、フィルタリングロジックを Query Object パターン（rails-patterns gem）やスコープチェーンを返す専用メソッドに抽出することで、複雑度を下げられる可能性があります。コーディングガイドラインでは、複雑な ActiveRecord クエリには QueryObject パターンの検討が推奨されています。
現時点では問題ありませんが、将来的なリファクタリングの候補として検討してください。
app/models/report.rb (1)
64-66: スコープのロジックは正しいですが、クエリの効率化を検討してください。このスコープは、
source_idがnilのプラクティスを持つ日報を除外することで、給付金コースの日報のみを取得します。ロジック自体は正しいです。ただし、サブクエリを使用しているため、データ量が増えた場合にパフォーマンスへの影響が考えられます。将来的には
NOT EXISTSを使ったアプローチも検討できます：♻️ 代替案（参考）
scope :with_grant_practices, lambda { joins(:practices) .where.not( Practice.joins(:reports) .where('reports.id = reports_practices.report_id') .where(source_id: nil) .arel.exists ) .distinct }または
where.associatedと組み合わせたアプローチも可能です。現時点では問題ありませんが、将来のパフォーマンス最適化の候補としてご検討ください。
app/javascript/components/Reports.jsx (1)
37-57:useEffectの依存配列にdataを含める必要性を確認してください。
dataを依存配列に含めると、データが取得されるたびに effect が再実行され、grant filter が再レンダリングされます。これは意図的な動作かもしれませんが（DOM要素が存在するのを待つため）、isGrantCourseが変更されない限り、filter UIを再初期化する必要はないはずです。また、
isGrantCourseはpracticeSourceIdから派生していますが、依存配列には含まれていません。ESLintのreact-hooks/exhaustive-depsルールに従うなら、isGrantCourseまたはpracticeSourceIdを依存配列に追加することを検討してください。♻️ 検討案
useEffect(() => { if (!isGrantCourse) return let grantFilter = null const initGrantFilter = async () => { const targetElement = document.querySelector('[data-grant-filter]') if (!targetElement) return const GrantFilter = (await import('../grant-filter')).default grantFilter = new GrantFilter(withGrant, setWithGrant) grantFilter.render(targetElement) } initGrantFilter() return () => { if (grantFilter) { grantFilter.destroy() grantFilter = null } } - }, [data, withGrant]) + }, [isGrantCourse, withGrant])
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.rubocop.ymlapp/controllers/api/reports_controller.rbapp/javascript/components/Reports.jsxapp/javascript/grant-filter.jsapp/models/report.rbapp/views/practices/reports/index.html.slimdb/fixtures/courses_categories.ymldb/fixtures/discord_profiles.ymldb/fixtures/reports.ymldb/fixtures/talks.ymldb/fixtures/users.yml
💤 Files with no reviewable changes (1)
- db/fixtures/courses_categories.yml
🧰 Additional context used
📓 Path-based instructions (5)
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
app/**/*.rb: Place Rails app code inapp/directory with subdirectories formodels/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/
Follow Rails file naming conventions (e.g.,app/models/user.rb)
Files:
app/controllers/api/reports_controller.rbapp/models/report.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indent for Ruby code, snake_case for method names, and CamelCase for class names as enforced by RuboCop
Files:
app/controllers/api/reports_controller.rbapp/models/report.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/api/reports_controller.rbapp/models/report.rb
app/views/**/*.slim
📄 CodeRabbit inference engine (AGENTS.md)
Place Slim templates in
app/views/and ensure they are linted according toconfig/slim_lint.yml
Files:
app/views/practices/reports/index.html.slim
app/javascript/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
app/javascript/**/*.{js,ts,jsx,tsx}: Place JavaScript/TypeScript code inapp/javascript/and lint with ESLint + Prettier
Use React 18 and Shakapacker/Webpack 5 for frontend JavaScript/TypeScript
Files:
app/javascript/grant-filter.jsapp/javascript/components/Reports.jsx
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: - どうしても避けられない時以外に新規にVue.js, Reactのコードを追加しない。
- fetchメソッドの代わりにrequest.jsを使う。
Files:
app/javascript/grant-filter.js
🧠 Learnings (7)
📓 Common learnings
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/product.rb:59-61
Timestamp: 2025-09-11T14:47:53.256Z
Learning: Rails アップグレードPRでは、アップグレードに直接関連しない性能改善や機能追加の提案は避けるべき。PRのスコープを維持することが重要で、そのような改善提案は別PRで対応すべきである。
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 8992
File: app/models/correct_answer_notifier.rb:23-23
Timestamp: 2025-07-30T07:26:36.599Z
Learning: PRのスコープを維持することが重要であり、バグ修正PRにおいて、修正対象以外の機能（例：ルーティングテスト）の追加提案は適切ではない。そのような改善が必要な場合は別PRで対応すべきである。
Learnt from: smallmonkeykey
Repo: fjordllc/bootcamp PR: 9314
File: app/helpers/page_tabs/dashboard_helper.rb:8-12
Timestamp: 2025-11-19T07:55:51.504Z
Learning: fjordllc/bootcamp プロジェクトにおいて、分報（micro_reports）は current_user 専用の機能ではなく、他ユーザーの分報も閲覧・管理する UI で使われているため、ダッシュボードの「自分の分報」タブでは user_micro_reports_path(current_user, ...) を使用し、/users/:id/micro_reports の URL 構造を維持する。current_user 名前空間に新しいルートを追加しない方針である。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:52-56
Timestamp: 2025-09-12T21:17:45.295Z
Learning: Rails upgrade PRにおいて、configuration systemの改善提案（shared configuration fallback等）も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
📚 Learning: 2026-01-08T21:12:18.350Z
Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 8566
File: app/controllers/pair_works_controller.rb:10-18
Timestamp: 2026-01-08T21:12:18.350Z
Learning: In fjordllc/bootcamp, when using ActiveRecord.order to specify ascending order, omit the explicit :asc and rely on Rails' default ASC behavior. Prefer .order(:published_at) over .order(published_at: :asc). If you need a non-default order (e.g., DESC), specify it explicitly. This guideline applies to Ruby/Rails controller and model files, especially in app/controllers and similar ActiveRecord usage sites.
Applied to files:
app/controllers/api/reports_controller.rb
📚 Learning: 2025-08-24T13:40:16.118Z
Learnt from: sjabcdefin
Repo: fjordllc/bootcamp PR: 9090
File: app/javascript/user_sns.js:36-45
Timestamp: 2025-08-24T13:40:16.118Z
Learning: app/javascript/user_sns.js では baseUrl は固定値、url は db/fixtures/users.yml で定義されているフィクスチャデータを使用している
Applied to files:
db/fixtures/users.yml
📚 Learning: 2025-08-24T13:40:16.118Z
Learnt from: sjabcdefin
Repo: fjordllc/bootcamp PR: 9090
File: app/javascript/user_sns.js:36-45
Timestamp: 2025-08-24T13:40:16.118Z
Learning: app/javascript/user_sns.js で使用される SNS フィールド（github_account, twitter_account, facebook_url, blog_url）は db/fixtures/users.yml の読み取り専用データで、ユーザーや管理者による編集機能は存在しない
Applied to files:
db/fixtures/users.yml
📚 Learning: 2026-01-11T04:41:41.597Z
Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 8566
File: app/views/pair_works/_body.html.slim:95-100
Timestamp: 2026-01-11T04:41:41.597Z
Learning: Ruby 3.1以降では、ハッシュ値省略記法により、render 'schedule', pair_work: という形式が有効です。これは render 'schedule', pair_work: pair_work と等価で、同名の変数がスコープ内に存在する場合にのみ使用できます。fjordllc/bootcamp プロジェクトではこのモダンな Ruby 構文が使用されているため、構文エラーとして誤って指摘しないことを推奨します。対象ファイルは Slim テンプレート（例: app/views/.../*.slim）全体に適用可能です。
Applied to files:
app/views/practices/reports/index.html.slim
📚 Learning: 2026-01-09T10:57:42.506Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:57:42.506Z
Learning: Applies to app/**/*.rb : Place Rails app code in `app/` directory with subdirectories for `models/`, `controllers/`, `views/`, `jobs/`, `helpers/`, and frontend code under `javascript/`
Applied to files:
.rubocop.yml
📚 Learning: 2026-01-09T10:57:42.506Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:57:42.506Z
Learning: Applies to app/**/*.rb : Follow Rails file naming conventions (e.g., `app/models/user.rb`)
Applied to files:
.rubocop.yml
🧬 Code graph analysis (2)
app/javascript/grant-filter.js (1)
app/javascript/components/Reports.jsx (1)
withGrant(25-25)
app/javascript/components/Reports.jsx (1)
app/javascript/user_mentor_memo.js (1)
userId(9-9)
⏰ 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 (7)
app/controllers/api/reports_controller.rb (1)
11-11: LGTM!
with_grantパラメータによるフィルタリングの実装は適切です。文字列比較== 'true'はクエリパラメータの処理として正しく、既存のフィルタチェーンとの組み合わせも問題ありません。db/fixtures/discord_profiles.yml (1)
339-342: LGTM!
grant-courseユーザーに対応する Discord プロファイルフィクスチャが適切に追加されています。users.ymlの冒頭コメント（「userを増やした場合、以下のymlにも追記する」）に従った正しい対応です。db/fixtures/talks.yml (1)
238-241: LGTM!新しい
talk_grant_courseフィクスチャは既存のパターンに従っており、grant-courseユーザーと正しく関連付けられています。db/fixtures/reports.yml (1)
509-515:practicesの関連付けがない点を確認してください。給付金コースのフィルター機能をテストするためには、このレポートが給付金コースのプラクティス（
source_idを持つプラクティス）に関連付けられている必要がある可能性があります。with_grant_practicesスコープはpractices.source_id IS NOT NULLでフィルタリングするため、プラクティスの関連付けがないとフィルター機能のテストが正しく動作しない可能性があります。app/javascript/grant-filter.js (1)
47-56: LGTM!
destroy()メソッドでのイベントリスナーの削除と状態のクリーンアップは適切に実装されています。メモリリークを防ぐ良い実装です。app/javascript/components/Reports.jsx (1)
72-89: LGTM!grant filter のプレースホルダー要素の条件付きレンダリングは正しく実装されています。空の状態と結果がある状態の両方でフィルターが表示されるようになっています。
app/views/practices/reports/index.html.slim (1)
14-14: LGTM!
practiceSourceId:@practice.source_id`` の追加により、React コンポーネントが給付金コースかどうかを判断できるようになります。source_idが `nil` の場合は通常のプラクティス、値がある場合は給付金コースとして正しく処理されます。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
karlley
force-pushed
the
feature/reports-grant-course-filter
branch
from
088ac70 to
b195053
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/fixtures/reports.yml`:
- Around line 362-383: The new fixtures report76, report77 and report78 are
missing the reported_on field which can break date-dependent logic and ordering;
update each report entry (report76, report77, report78) to include a fixed
reported_on date (e.g. "2026-01-01") matching created_at so tests have a
deterministic report date.
c9e9e95 to
2e30b6a
Compare
|
@machida |
|
2点確認です。 1. 給付金コースのプラクティスの初期データについて給付金コースのプラクティスは 2. rubocopの警告への対応についてコントローラーに処理を追加したところ以下の警告が発生するようになりました。 app/controllers/api/reports_controller.rb:4:3: C: Metrics/PerceivedComplexity: Perceived complexity for index is too high. [9/8]
def index ...
^^^^^^^^^コントローラの分岐数が多いと発生する警告のようでした。 警告に対応するためには既存処理を修正する必要があり、当issueのスコープを外れると考えたため別issueでの対応を検討しています。 |
こちら、そういうわけではないです。 |
承知しました。 開発環境環境用の給付金コースの以下のテストデータはrakeタスクを使用せずに用意するように手順を変更した方が良いでしょうか？ また、以下についても回答いただけると幸いです。
よろしくお願いいたします。 |
karlley
force-pushed
the
feature/reports-grant-course-filter
branch
from
2e30b6a to
8223bc9
Compare
|
@karlley デザインはこれでOKですー👍対応ありがとうございます！！ |
はい。seedデータとして最初から用意されてる感じが良いと思います。
はい、お願い致します。 |
karlley
force-pushed
the
feature/reports-grant-course-filter
branch
3 times, most recently
from
d38a577 to
a2574bf
Compare
karlley
force-pushed
the
feature/reports-grant-course-filter
branch
3 times, most recently
from
81a34ab to
b84af96
Compare
prettierでの修正
karlley
force-pushed
the
feature/reports-grant-course-filter
branch
from
ceedf65 to
bc02612
Compare
|
お疲れ様です！ |
|
@karlley ポイント割当をしました〜 |
|
@karlley 私も給付金コースの関連のissue(Docs)を担当しているのですが、karlleyさんの日報を取得するクエリの実装を追っていたところ、 karlleyさんのアプローチ 下記が給付金コース以外のものを削るものかと思っています。 scope :without_copied_practice, lambda {
joins(:practices).where.not(id: Report.joins(:practices).where(practices: { source_id: nil })).distinct
}私のアプローチ 恐らくkarlleyさんが上記のアプローチで実装されたのは もちろん異論や、私が把握できていない仕様・懸念点等あるかと思います。Discordでの会話も可能なので、一度相談させていただきたいです。 |
|
また、ローカルでこちらのブランチを動かして、元コースに日報を追加したところ。 給付金コースのほうの日報一覧に表示されませんでした。 恐らく現状日報に給付金コースのタグが付いていないと表示されない仕組みとなっているかと思います。 |
|
@karlley @kutimiti1234 |
|
@komagata |
|
確認ありがとうございます！
そうですね。私もこちらの方が良いと思います。
私の実装が既存日報しか考慮できてないかもしれません。 また、djkazunokoさんのコメントにあるようにkutimiti1234さんが問題なければレビューまでお願いしたいと思っているのですが可能でしょうか？ |
|
@karlley レビューの件承知しました。連絡お待ちしております。 |
karlley
force-pushed
the
feature/reports-grant-course-filter
branch
3 times, most recently
from
578d1cc to
40b59aa
Compare
karlley
force-pushed
the
feature/reports-grant-course-filter
branch
from
40b59aa to
4341470
Compare
5 participants
Issue
前提
wiki: 給付金コースの人が元コースの関連情報を見れるようにする
概要
給付金コースのプラクティスの関連日報一覧(/practices/:id/reports)に元コースの関連日報の表示/非表示を切り替えるフィルターを追加
全て: コピー元プラクティス + 給付金コース給付金コース: 給付金コースのみrobocopの警告への対応について
処理を追加したことでrubocopの警告への対応が必要でしたが当issueのスコープを外れると判断しました。
そのため、当PRでは暫定対応としてコメントアウト、恒久対応として別issueでリファクタリングを行います。
変更確認方法
事前準備
feature/reports-grant-course-filterをローカルに取り込むフィルター表示/非表示
komagataでログインgrant-courseでログインフィルタリング
grant-courseでログイン全て: コピー元 + 給付金コース給付金コース: 給付金コースのみ給付金が日報が0件時の表示
grant-courseでログインScreenshot
変更前
変更後
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.