-
Notifications
You must be signed in to change notification settings - Fork 75
Feature/reports course type filter #9450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthrough給付金コース向けのレポートフィルタリング機能を実装。React UIでタブ切り替え可能なフィルタを追加し、バックエンドのControllerとModelで対応するスコープを実装。テストデータ用のfixturesも充実。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as ユーザー
participant React as React<br/>(Reports)
participant GF as GrantFilter<br/>UIコンポーネント
participant API as API<br/>Controller
participant DB as Database<br/>(Model)
User->>React: ページロード
React->>GF: grant-filterコンポーネントを<br/>DOMマウント
GF->>GF: 初期状態を表示<br/>(withGrant値に基づく)
User->>GF: タブをクリック<br/>(全て or 給付金コース)
GF->>GF: アクティブタブを更新
GF->>React: setWithGrant callback呼び出し<br/>(true/false)
React->>React: withGrant状態を更新
React->>API: with_grant パラメータ付きで<br/>API呼び出し
API->>DB: with_grant='true'の場合<br/>with_grant_practices スコープを適用
DB->>DB: source_idがnil でない<br/>practicesを持つレポートのみ抽出
DB->>API: フィルタ済みレポート返却
API->>React: レポート一覧JSON
React->>User: フィルタ済みレポート表示
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
prettierでの修正
7e347c5 to
c1b6e44
Compare
c1b6e44 to
9809e11
Compare
9c9987e to
c73d17e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/javascript/components/Reports.jsx`:
- Around line 37-57: The useEffect currently depends on [data, withGrant]
causing unnecessary re-initialization; change the dependency array to
[isGrantCourse] (so the grantFilter initializes only when the course type
toggles) and remove data and withGrant from the deps; keep the internal
initGrantFilter logic as-is (or move grantFilter to a ref if you need it to
persist across renders) and ensure GrantFilter consumes setWithGrant for state
sync (or is updated to accept an initial withGrant without requiring effect
re-creation).
In `@app/javascript/grant-filter.js`:
- Around line 9-21: The template string in render(element) has a malformed
closing tag ("</nav" instead of "</nav>") causing HTML parse errors; update the
template literal so the nav element is properly closed with "</nav>" inside the
render(element) method where the markup string is constructed.
🧹 Nitpick comments (2)
.rubocop.yml (1)
32-34: RuboCop除外の追加について検討してください。
Metrics/PerceivedComplexityの除外を追加するのではなく、コーディングガイドラインに従ってQueryObjectパターン(rails-patternsgemの Query 機能)を使用してコントローラーのロジックをリファクタリングすることを検討してください。これにより、フィルタリングロジックを再利用可能で、テストしやすい形で分離できます。ただし、現状の複雑さは既存のコントローラーパターンに沿っており、許容範囲内です。
db/fixtures/users.yml (1)
1559-1577:name_kanaフィールドがカタカナではなく漢字になっています。他のユーザーフィクスチャでは
name_kanaはカタカナで記載されています(例: "コマガタ マサキ"、"キムラ タダシ")。一貫性のため、カタカナ表記に修正することを推奨します。📝 修正案
grant-course: login_name: grant-course email: [email protected] crypted_password: $2a$10$n/xv4/1luueN6plzm2OyDezWlZFyGHjQEf4hwAW1r3k.lCm0frPK. # testtest salt: zW3kQ9ubsxQQtzzzs4ap name: 給付金コースのユーザー - name_kana: 給付金コースのユーザー + name_kana: キュウフキンコースノユーザー github_account: grant-course
📜 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モデルを操作する1つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。(interactor gemを導入しているのでそれを使う)
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。(interactor gemを導入しており、その中にOrganizerの機能があるので使う)
Files:
app/controllers/api/reports_controller.rbapp/models/report.rb
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
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
🧠 Learnings (8)
📓 Common learnings
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: jun-kondo
Repo: fjordllc/bootcamp PR: 8977
File: app/controllers/reports_controller.rb:63-63
Timestamp: 2025-07-23T20:31:13.856Z
Learning: fjordllc/bootcampプロジェクトの`app/controllers/reports_controller.rb`において、`create`と`update`アクションは両方とも`report.save_uniquely`を使用し、同じ`:report_save`イベントと`'report.save'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
📚 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
📚 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: 2025-11-13T20:44:13.908Z
Learnt from: karlley
Repo: fjordllc/bootcamp PR: 9304
File: app/javascript/practice-filter-dropdown.js:3-3
Timestamp: 2025-11-13T20:44:13.908Z
Learning: fjordllc/bootcampプロジェクトでは、JavaScriptファイルでクラスをエクスポートする際に`export default class {`のパターン(無名クラス)を一貫して使用している。これは複数のファイル(textarea-autocomplte-mention.js、textarea-autocomplte-emoji.js、user-icon-renderer.js等)で確認されており、プロジェクトのコーディング規約となっている。
Applied to files:
app/javascript/grant-filter.js
📚 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
🧬 Code graph analysis (1)
app/javascript/grant-filter.js (1)
app/javascript/components/Reports.jsx (1)
withGrant(25-25)
🔇 Additional comments (8)
app/controllers/api/reports_controller.rb (1)
11-11: LGTM!既存のスコープチェーンパターンに従っており、
params[:with_grant] == 'true'の文字列比較はクエリパラメータの標準的な扱い方として適切です。db/fixtures/talks.yml (1)
239-241: LGTM!既存のフィクスチャパターンに従っており、
grant-courseユーザー用のtalkエントリが適切に追加されています。app/models/report.rb (1)
64-66: NOT EXISTS の代替案は Rails では機能しません。提案されているコード例
where.not(...arel.exists)は Rails では無効な構文です。このスコープのロジック自体は正しく機能していますが、パフォーマンス最適化が必要な場合は、別 PR で適切な代替実装(例:LEFT JOINを使用した代替案など)を検討してください。Likely an incorrect or invalid review comment.
db/fixtures/reports.yml (1)
509-515:practicesフィールドの追加を確認してください。
with_grant_practicesスコープはjoins(:practices)を使用するため、practices に関連付けられていないレポートは結果に含まれません。このフィクスチャにはpracticesフィールドがないため、給付金コースフィルターでこのレポートが正しく取得されない可能性があります。他のレポートフィクスチャ(例: report1, report17, report21, report22)では
practicesフィールドが設定されています。⛔ Skipped due to learnings
Learnt from: sharoa119 Repo: fjordllc/bootcamp PR: 9012 File: app/queries/grass_learning_time_query.rb:30-37 Timestamp: 2025-09-05T03:16:53.387Z Learning: reports テーブルには既に (user_id, reported_on) の一意制約インデックス index_reports_on_user_id_and_reported_on が存在するため、学習時間集計クエリのパフォーマンス最適化には追加のインデックスは不要。Learnt from: sharoa119 Repo: fjordllc/bootcamp PR: 9012 File: db/migrate/20250905025850_remove_redundant_index_from_reports.rb:5-5 Timestamp: 2025-09-05T03:33:31.659Z Learning: reports テーブルには既存の一意制約インデックス index_reports_on_user_id_and_reported_on があるため、同じカラム組み合わせ (user_id, reported_on) の非一意インデックスは冗長であり削除が適切。Learnt from: jun-kondo Repo: fjordllc/bootcamp PR: 8977 File: app/controllers/reports_controller.rb:63-63 Timestamp: 2025-07-23T20:31:13.856Z Learning: fjordllc/bootcampプロジェクトの`app/controllers/reports_controller.rb`において、`create`と`update`アクションは両方とも`report.save_uniquely`を使用し、同じ`:report_save`イベントと`'report.save'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。app/views/practices/reports/index.html.slim (1)
14-14: LGTM!
practiceSourceIdプロパティを追加することで、給付金コースフィルター機能が適切に有効化されます。Reportsコンポーネントとの連携も正しく行われています。db/fixtures/discord_profiles.yml (1)
339-342: LGTM!
grant-courseユーザーに対応するDiscordプロフィールフィクスチャが正しく追加されています。users.ymlのコメントで指示されている通り、ユーザー追加時にdiscord_profiles.ymlも更新されています。app/javascript/components/Reports.jsx (1)
76-76: 給付金コースフィルターのプレースホルダー配置は適切です。
data.totalPages === 0とdata.totalPages > 0の両方のケースでフィルターが表示されるようになっており、レポートが存在しない場合でもフィルタリングUIが利用可能です。Also applies to: 89-89
app/javascript/grant-filter.js (1)
1-7: クラス構造とクリーンアップ処理は適切です。プロジェクトの規約に従った無名クラスのエクスポートパターンを使用しており、
destroy()メソッドでイベントリスナーの解除と内部状態のクリアが適切に実装されています。Based on learnings, このパターンはfjordllc/bootcampプロジェクトのコーディング規約に沿っています。Also applies to: 47-56
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| 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]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useEffect の依存配列を見直してください。
現在の依存配列 [data, withGrant] には問題があります:
dataはフェッチごとに変わるため、不要な再レンダリングとgrantFilterの再初期化が発生します。withGrantが変わるとフィルターが破棄・再作成され、タブ切り替え時にUIがちらつく可能性があります。
grantFilterは初回マウント時に一度だけ初期化すれば十分です。withGrantの状態はsetWithGrantを通じてReactが管理するため、フィルター自体を再作成する必要はありません。
🔧 修正案
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 の初期値を渡す必要がある場合は、GrantFilter クラス側で setWithGrant を使った状態同期の仕組みを検討してください。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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]) | |
| 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 | |
| } | |
| } | |
| }, [isGrantCourse]) |
🤖 Prompt for AI Agents
In `@app/javascript/components/Reports.jsx` around lines 37 - 57, The useEffect
currently depends on [data, withGrant] causing unnecessary re-initialization;
change the dependency array to [isGrantCourse] (so the grantFilter initializes
only when the course type toggles) and remove data and withGrant from the deps;
keep the internal initGrantFilter logic as-is (or move grantFilter to a ref if
you need it to persist across renders) and ensure GrantFilter consumes
setWithGrant for state sync (or is updated to accept an initial withGrant
without requiring effect re-creation).
| render(element) { | ||
| element.innerHTML = ` | ||
| <nav class="pill-nav"> | ||
| <ul class="pill-nav__items"> | ||
| <li class="pill-nav__item"> | ||
| <button class="pill-nav__item-link" data-with-grant="false">全て</button> | ||
| </li> | ||
| <li class="pill-nav__item"> | ||
| <button class="pill-nav__item-link" data-with-grant="true">給付金コース</button> | ||
| </li> | ||
| </ul> | ||
| </nav | ||
| ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTMLの構文エラー: </nav の閉じタグが不完全です。
Line 20で </nav となっていますが、正しくは </nav> です。このエラーによりDOMパースが正しく行われず、UIが壊れる可能性があります。
🐛 修正案
element.innerHTML = `
<nav class="pill-nav">
<ul class="pill-nav__items">
<li class="pill-nav__item">
<button class="pill-nav__item-link" data-with-grant="false">全て</button>
</li>
<li class="pill-nav__item">
<button class="pill-nav__item-link" data-with-grant="true">給付金コース</button>
</li>
</ul>
- </nav
+ </nav>
`🤖 Prompt for AI Agents
In `@app/javascript/grant-filter.js` around lines 9 - 21, The template string in
render(element) has a malformed closing tag ("</nav" instead of "</nav>")
causing HTML parse errors; update the template literal so the nav element is
properly closed with "</nav>" inside the render(element) method where the markup
string is constructed.
Issue
概要
変更確認方法
Screenshot
変更前
変更後
Summary by CodeRabbit
リリースノート
新機能
その他
✏️ Tip: You can customize this high-level summary in your review settings.