OAuth APIをpjord向けに拡張(全API Doorkeeper対応 + 研修生進捗エンドポイント)#9830
Conversation
- BaseControllerにDoorkeeper OAuth認証を統合(全APIエンドポイント対応) - CommentsController/UsersControllerの重複doorkeeper設定を整理 - 日報APIレスポンスにdescription(本文)を追加 - 日報showアクション追加(個別日報+コメント+プラクティス取得) - 企業研修生進捗エンドポイント追加(GET /api/trainee_progresses)
- week_startパラメータで対象週を指定可(デフォルト: 今週) - 週次アクティビティ: 日報提出日数/5日、提出日一覧 - プラクティスステータス変化の一覧 - 現在取り組み中プラクティスの滞留日数 - 全体進捗(完了率、完了/必須プラクティス数)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAPI 認可の条件付き統合を BaseController に導入、Comments/Users の認証コールバックを整理。Reports に 変更内容
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
詩
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
app/views/api/trainee_progresses/index.json.jbuilder (2)
67-71:UserCoursePracticeによる追加クエリの発生に注意してください。
UserCoursePracticeは各研修生ごとにインスタンス化され、completed_required_practices.size、required_practices.size、completed_percentageの呼び出しで個別にDBクエリが発行されます。研修生数が多い場合、パフォーマンスに大きな影響を与える可能性があります。現時点で大規模なリファクタリングは必要ないかもしれませんが、研修生数が増加した際にはバッチクエリやキャッシュの検討をお勧めします。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/api/trainee_progresses/index.json.jbuilder` around lines 67 - 71, The view is causing N+1 queries by calling UserCoursePractice methods (completed_required_practices, required_practices, completed_percentage) per trainee; fix by precomputing or eager-loading the needed data in the controller and passing simple primitives to the template (e.g., a collection like user_course_practice_summaries with completed_count, required_count, completed_percentage for each trainee) or by eager-loading the associations used by UserCoursePractice so those method calls do not hit the DB; update the controller that builds user_course_practices and the template to read those precomputed attributes instead of calling the methods directly.
14-23:learningsへの重複クエリを統合できます。現在、
trainee.learningsに対して2回別々にクエリを発行しています:
- Lines 14-17: 週次の学習状況取得
- Lines 20-23: 現在取り組み中のプラクティス取得
これらをRubyで処理することで、クエリ数を削減できます。
♻️ クエリを統合する修正案
- # 対象週にステータスが変化したlearning - week_learnings = trainee.learnings - .where(updated_at: `@week_start.beginning_of_day`..@week_end.end_of_day) - .where.not(status: :unstarted) - .includes(:practice) - - # 現在取り組み中のプラクティスの滞留日数 - current_learning = trainee.learnings - .where(status: :started) - .order(updated_at: :desc) - .first + # learningsを一度取得してフィルタリング + all_learnings = trainee.learnings.includes(:practice).to_a + + week_learnings = all_learnings.select do |l| + l.updated_at >= `@week_start.beginning_of_day` && + l.updated_at <= `@week_end.end_of_day` && + l.status != 'unstarted' + end + + current_learning = all_learnings + .select { |l| l.status == 'started' } + .max_by(&:updated_at)注意: この最適化はコントローラー側で
learnings: :practiceをプリロードしている前提です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/api/trainee_progresses/index.json.jbuilder` around lines 14 - 23, Querying trainee.learnings twice is redundant; fetch once into a Ruby array and derive both results from it: replace the two separate queries with a single load like all_learnings = trainee.learnings.includes(:practice).where.not(status: :unstarted).to_a, then compute week_learnings = all_learnings.select { |l| l.updated_at.between?(`@week_start.beginning_of_day`, `@week_end.end_of_day`) } and current_learning = all_learnings.select { |l| l.status == "started" }.max_by(&:updated_at) (or .find / .max_by as appropriate) so no duplicate DB calls are made while keeping the existing variable names week_learnings and current_learning.app/views/api/reports/show.json.jbuilder (1)
13-13: 昇順の明示的な指定は省略可能です。プロジェクトの慣例として、昇順(ASC)はRailsのデフォルト動作に依存し、明示的な
:asc指定を省略することが推奨されています。♻️ 修正案
-json.comments `@report.comments.order`(created_at: :asc) do |comment| +json.comments `@report.comments.order`(:created_at) do |comment|Based on learnings: "In fjordllc/bootcamp, when using ActiveRecord.order to specify ascending order, omit the explicit :asc and rely on Rails' default ASC behavior."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/api/reports/show.json.jbuilder` at line 13, 該当行で明示的な昇順指定をしているため冗長です:json.comments `@report.comments.order`(created_at: :asc) do |comment|。慣例に合わせて明示的な :asc を削除し、@report.comments.order(:created_at) のように Rails のデフォルト ASC に依存する形に置き換えてください(対象シンボル:@report.comments.order と created_at)。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/api/reports_controller.rb`:
- Around line 17-19: showアクションで単純に Report.find を使っているため、show.json.jbuilder
で参照している `@report.user`、@report.practices、@report.comments(および各
comment.user)、@report.checks(および checks.last.user.login_name)に対してN+1が発生します。修正は
show 内の Report.find(params[:id]) を関連をプリロードするように変更し、includes で
user、practices、comments: :user、checks: :user を指定して `@report`
を取得するようにしてください(メソッド名:show、変数:@report、参照箇所:@report.user/@report.practices/@report.comments->comment.user/@report.checks->checks.last.user.login_name
を目安に実装)。
In `@app/controllers/api/trainee_progresses_controller.rb`:
- Around line 5-7: index.json.jbuilder is accessing trainee.reports and
trainee.learnings causing N+1 queries; update the controller where `@trainees` is
built (the User.trainees preload chain that sets `@trainees`) to include :reports
and :learnings in the preload call (in addition to :company, :course,
:avatar_attachment, :active_practices) so those associations are eager-loaded
for the index view, or if concerned about overhead for large trainee sets,
implement a scoped/preloaded query for only the date ranges used by
index.json.jbuilder.
- Around line 10-14: Wrap the Date.parse call for params[:week_start] in a
begin/rescue that catches ArgumentError and Date::Error, and handle invalid
input by returning a 400 Bad Request (render json with an error message) or
falling back to Date.current.beginning_of_week; specifically update the code
that sets `@week_start` (the Date.parse(params[:week_start]) branch) to rescue
those exceptions, use API::BaseController-compatible response handling (e.g.,
render json: { error: "invalid week_start" }, status: :bad_request) or assign
the safe default, and optionally log the invalid params[:week_start] value for
debugging.
In `@app/views/api/trainee_progresses/index.json.jbuilder`:
- Around line 37-40: The json.course block accesses trainee.course.id without
guarding against nil; wrap the course rendering in the same style as company
(e.g., check if trainee.course.present? before the json.course do block) or use
safe navigation (trainee.course&.id / trainee.course&.title) so NoMethodError is
avoided; update the view in index.json.jbuilder around the json.course block to
conditionally render or output nil when trainee.course is absent.
- Around line 56-64: The view accesses current_learning.practice.title without
preloading practice, which can cause N+1 queries; modify the code that builds
current_learning (the query that sets current_learning) to include the
association (use .includes(:practice)) so practice is eager-loaded, mirroring
how week_learnings is already loaded with .includes(:practice).
---
Nitpick comments:
In `@app/views/api/reports/show.json.jbuilder`:
- Line 13: 該当行で明示的な昇順指定をしているため冗長です:json.comments
`@report.comments.order`(created_at: :asc) do |comment|。慣例に合わせて明示的な :asc
を削除し、@report.comments.order(:created_at) のように Rails のデフォルト ASC
に依存する形に置き換えてください(対象シンボル:@report.comments.order と created_at)。
In `@app/views/api/trainee_progresses/index.json.jbuilder`:
- Around line 67-71: The view is causing N+1 queries by calling
UserCoursePractice methods (completed_required_practices, required_practices,
completed_percentage) per trainee; fix by precomputing or eager-loading the
needed data in the controller and passing simple primitives to the template
(e.g., a collection like user_course_practice_summaries with completed_count,
required_count, completed_percentage for each trainee) or by eager-loading the
associations used by UserCoursePractice so those method calls do not hit the DB;
update the controller that builds user_course_practices and the template to read
those precomputed attributes instead of calling the methods directly.
- Around line 14-23: Querying trainee.learnings twice is redundant; fetch once
into a Ruby array and derive both results from it: replace the two separate
queries with a single load like all_learnings =
trainee.learnings.includes(:practice).where.not(status: :unstarted).to_a, then
compute week_learnings = all_learnings.select { |l|
l.updated_at.between?(`@week_start.beginning_of_day`, `@week_end.end_of_day`) } and
current_learning = all_learnings.select { |l| l.status == "started"
}.max_by(&:updated_at) (or .find / .max_by as appropriate) so no duplicate DB
calls are made while keeping the existing variable names week_learnings and
current_learning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 315e1039-ad96-4bfd-87a3-3c7f8ab6dd73
📒 Files selected for processing (9)
app/controllers/api/base_controller.rbapp/controllers/api/comments_controller.rbapp/controllers/api/reports_controller.rbapp/controllers/api/trainee_progresses_controller.rbapp/controllers/api/users_controller.rbapp/views/api/reports/_report.json.jbuilderapp/views/api/reports/show.json.jbuilderapp/views/api/trainee_progresses/index.json.jbuilderconfig/routes/api.rb
💤 Files with no reviewable changes (2)
- app/controllers/api/comments_controller.rb
- app/controllers/api/users_controller.rb
🚀 Review AppURL: https://bootcamp-pr-9830-fvlfu45apq-an.a.run.app
|
- reports#show: includes で user, practices, checks, comments を プリロード(N+1解消) - trainee_progresses: reports, learnings をpreload(N+1解消) - trainee_progresses: learningsクエリを統合(Rubyフィルタリングに変更) - trainee_progresses: Date.parseのエラーハンドリング追加 - trainee_progresses: course nilガード追加 - reports/show: order(:created_at) のasc省略(Rails慣例に準拠)
|
CodeRabbitのレビュー指摘に全て対応しました (e533ede):
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/api/trainee_progresses_controller.rb`:
- Around line 4-9: The index action in trainee_progresses_controller.rb
currently returns all trainees without role checks; add an authorization guard
that restricts access to admins or mentors by invoking the same authorization
used in API::ReportsController (e.g., call admin_or_mentor? or
current_user.admin_or_mentor? at the start of the index method), and
return/raise an unauthorized response if the check fails before executing
User.trainees/preload/order/where; ensure the authorization check is applied to
the index method only and mirrors the behavior/response shape used in
API::ReportsController.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8a0ab14-3ac4-4105-8c63-1381857e7c15
📒 Files selected for processing (4)
app/controllers/api/reports_controller.rbapp/controllers/api/trainee_progresses_controller.rbapp/views/api/reports/show.json.jbuilderapp/views/api/trainee_progresses/index.json.jbuilder
✅ Files skipped from review due to trivial changes (1)
- app/views/api/reports/show.json.jbuilder
🚧 Files skipped from review as they are similar to previous changes (2)
- app/controllers/api/reports_controller.rb
- app/views/api/trainee_progresses/index.json.jbuilder
showアクション追加により /api/reports/unchecked が reports#show(id: 'unchecked') として解釈されていた問題を修正。 namespace 'reports' を resources :reports より前に配置することで unchecked/recents のルートが優先されるようにした。
- ShadowedException: Date::ErrorはArgumentErrorのサブクラスなのでArgumentErrorのみrescue - StringLiterals: シングルクォートに統一 - FrozenStringLiteralComment追加 - InverseMethods: select+!=をrejectに変更 - Layout修正(インデント等) - AmbiguousBlockAssociation修正 - Metrics/BlockLength: jbuilderブロックはinline disableで対応
概要
pjord(AIアシスタント)がbootcampのデータにアクセスできるよう、APIをOAuth (Doorkeeper) 対応に拡張します。
変更内容
1. 全APIエンドポイントをOAuth対応
API::BaseControllerにdoorkeeper_authorize!を統合2. 日報API強化
GET /api/reports— レスポンスにdescription(本文)追加GET /api/reports/:id— 個別日報取得(コメント・プラクティス付き)3. 企業研修生進捗エンドポイント(新規)
GET /api/trainee_progresses— 全研修生の進捗一覧GET /api/trainee_progresses?company_id=123— 企業別フィルタGET /api/trainee_progresses?week_start=2026-03-16— 対象週指定関連
Summary by CodeRabbit
新機能
改善