Conversation
📝 WalkthroughWalkthroughmicro_reportにMentionerを組み込み、MicroReportにpath・senderエイリアス・削除コールバックを追加。Userに分報の順序取得と特定分報のページ算出を追加し、コントローラーは Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as ブラウザ
participant Controller as Users::MicroReportsController
participant UserModel as User
participant Router as Rails Redirect
Browser->>Controller: GET /users/:id/micro_reports?micro_report_id=:id
Controller->>UserModel: page_of_micro_report(micro_report_id, PAGER_NUMBER)
UserModel-->>Controller: page number or nil
alt found
Controller->>Router: redirect to /users/:id/micro_reports?page=N#micro_report_<id>
Router-->>Browser: 302 Redirect
else not found
Controller-->>Browser: redirect to index with alert
end
sequenceDiagram
participant App as Application
participant MicroReport as MicroReport
participant Callbacks as MicroReportCallbacks
participant Notification as Notification DB
App->>MicroReport: destroy
MicroReport->>Callbacks: after_destroy(micro_report)
Callbacks->>Notification: where(link: micro_report.path).destroy_all
Notification-->>Callbacks: destroyed records
Callbacks-->>App: callback complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
156cc26 to
d631aea
Compare
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
d631aea to
906e747
Compare
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
4 times, most recently
from
d285b98 to
d31acce
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/controllers/users/micro_reports_controller.rb (1)
11-18: 特定の分報へのリダイレクト機能が適切に実装されています。micro_report_idパラメータが指定された場合に、対象の分報が含まれるページへリダイレクトする機能が正しく実装されています。
ordered_micro_reportsの使用により、コードの可読性と保守性が向上しています。Line 13の
page_of_micro_reportの戻り値は理論上nilになる可能性があります。実際にはLine 12のfindが成功すれば常にページ番号が返されるはずですが、より防御的なコードにするため、以下のようなnil チェックを検討してもよいでしょう：if params[:micro_report_id].present? micro_report = @user.micro_reports.find(params[:micro_report_id]) page = @user.page_of_micro_report(params[:micro_report_id].to_i, PAGER_NUMBER) + page ||= FIRST_PAGE return redirect_to user_micro_reports_path(@user, page:, anchor: "micro_report_#{micro_report.id}") endただし、現状の実装でも実用上問題はなく、テストでカバーされているため、この変更は任意です。
app/models/micro_report.rb (1)
13-18: MicroReport#path の設計は要件と合っており妥当です（将来的な共通化案は任意）分報には個別の show ページがなく、ユーザーの分報一覧に
micro_report_idパラメータ付きで遷移する設計なので、このuser_micro_reports_path(user, micro_report_id: id)を返す実装は要件と噛み合っていると思います。
将来的に他の mentionable モデルでも似たような URL 生成が増えてきたら、ルーティングヘルパを使うpath実装を Concern などに共通化してもよさそうですが、現状のままでも十分シンプルで読みやすいです。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/controllers/users/micro_reports_controller.rb(1 hunks)app/models/concerns/mentioner.rb(1 hunks)app/models/micro_report.rb(1 hunks)app/models/user.rb(1 hunks)db/fixtures/micro_reports.yml(1 hunks)test/models/micro_report_test.rb(1 hunks)test/models/user_test.rb(1 hunks)test/system/notification/mention_test.rb(1 hunks)test/system/user/micro_reports_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
db/**
📄 CodeRabbit inference engine (AGENTS.md)
Database migrations and schema should be stored in
db/directory; app-specific utilities inlib/; static assets inpublic/
Files:
db/fixtures/micro_reports.yml
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/models/concerns/mentioner.rbapp/models/user.rbapp/models/micro_report.rbapp/controllers/users/micro_reports_controller.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
app/models/concerns/mentioner.rbtest/models/user_test.rbtest/models/micro_report_test.rbapp/models/user.rbapp/models/micro_report.rbtest/system/user/micro_reports_test.rbapp/controllers/users/micro_reports_controller.rbtest/system/notification/mention_test.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/models/concerns/mentioner.rbtest/models/user_test.rbtest/models/micro_report_test.rbapp/models/user.rbapp/models/micro_report.rbtest/system/user/micro_reports_test.rbapp/controllers/users/micro_reports_controller.rbtest/system/notification/mention_test.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.rb: Test suite should use Minitest with structure:system/,models/,controllers/, and fixtures intest/fixtures/; test files should be named*_test.rb
Use Minitest + Capybara for system tests; place unit and integration tests under matchingtest/*directories
Files:
test/models/user_test.rbtest/models/micro_report_test.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
test/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep tests deterministic and use fixtures stored in
test/fixtures/for test data
Files:
test/models/user_test.rbtest/models/micro_report_test.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/models/user_test.rbtest/models/micro_report_test.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
🧠 Learnings (7)
📓 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: 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: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: reckyy
Repo: fjordllc/bootcamp PR: 8356
File: app/models/notification.rb:45-47
Timestamp: 2025-07-15T12:58:02.735Z
Learning: fjordllc/bootcampプロジェクトの`app/models/notification.rb`において、`training_completed`通知種別は`TARGETS_TO_KINDS`ハッシュに含めない設計になっている。これは`training_completed`が通知一覧のフィルタ対象ではないためで、意図的な設計判断である。
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で対応すべきである。
📚 Learning: 2025-11-19T07:55:51.504Z
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 名前空間に新しいルートを追加しない方針である。
Applied to files:
db/fixtures/micro_reports.ymlapp/models/concerns/mentioner.rbtest/models/user_test.rbtest/models/micro_report_test.rbapp/models/user.rbapp/models/micro_report.rbtest/system/user/micro_reports_test.rbapp/controllers/users/micro_reports_controller.rbtest/system/notification/mention_test.rb
📚 Learning: 2025-07-10T12:59:27.807Z
Learnt from: kitarou888
Repo: fjordllc/bootcamp PR: 8277
File: test/models/searcher_test.rb:326-331
Timestamp: 2025-07-10T12:59:27.807Z
Learning: モデルテストにおいて、1つのクラスメソッドの複数の挙動を検証する場合、機能の異なる側面を同じテストメソッドでテストすることは、包括的なテストとして適切である。特に`only_me`のような機能では、異なる検索条件（空文字と具体的な検索語）を使い分けることで、より広範囲な動作保証が可能となる。
Applied to files:
test/models/user_test.rbtest/system/user/micro_reports_test.rb
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Use Minitest + Capybara for system tests; place unit and integration tests under matching `test/*` directories
Applied to files:
test/models/micro_report_test.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Test suite should use Minitest with structure: `system/`, `models/`, `controllers/`, and fixtures in `test/fixtures/`; test files should be named `*_test.rb`
Applied to files:
test/models/micro_report_test.rb
📚 Learning: 2025-09-04T01:27:23.422Z
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: db/fixtures/buzzes.yml:0-0
Timestamp: 2025-09-04T01:27:23.422Z
Learning: The buzz_controller.rb and related data migration files were deleted as part of the Buzz page redesign, not just modified. The old single buzz page was completely replaced with a new buzzes (plural) listing page with year-based navigation and metadata structure.
Applied to files:
app/controllers/users/micro_reports_controller.rb
📚 Learning: 2025-07-23T20:31:13.856Z
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'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Applied to files:
app/controllers/users/micro_reports_controller.rb
🧬 Code graph analysis (7)
test/models/user_test.rb (2)
test/models/micro_report_test.rb (1)
test(5-11)app/models/user.rb (2)
ordered_micro_reports(956-958)page_of_micro_report(960-966)
test/models/micro_report_test.rb (1)
app/models/micro_report.rb (1)
path(13-18)
app/models/user.rb (1)
app/controllers/users/micro_reports_controller.rb (1)
index(10-19)
app/models/micro_report.rb (2)
app/models/user.rb (1)
include(3-1029)test/system/notification/mention_test.rb (1)
include(7-94)
test/system/user/micro_reports_test.rb (1)
test/models/micro_report_test.rb (1)
test(5-11)
app/controllers/users/micro_reports_controller.rb (1)
app/models/user.rb (2)
page_of_micro_report(960-966)ordered_micro_reports(956-958)
test/system/notification/mention_test.rb (1)
test/supports/notification_helper.rb (1)
make_mention_notification_message(66-68)
⏰ 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 (10)
app/models/concerns/mentioner.rb (1)
27-28: LGTM!MicroReportのメンション機能が他の既存のケース（Product、Report等）と同じパターンで実装されており、一貫性があります。
test/system/notification/mention_test.rb (1)
80-93: LGTM!分報のメンション機能のシステムテストが適切に実装されています。既存のメンションテストと同じパターンに従っており、通知の作成から受信者での確認まで正しく検証されています。
db/fixtures/micro_reports.yml (1)
28-33: LGTM!ページネーションのテストに必要なフィクスチャデータが適切に追加されています。25件のデータ追加によりPAGER_NUMBERの境界値テストが可能になります。
test/models/user_test.rb (2)
761-774: LGTM!
ordered_micro_reportsメソッドのテストが適切に実装されています。created_atの昇順でソートされることを正しく検証しています。
776-790: LGTM!
page_of_micro_reportメソッドのページネーション計算ロジックが正しくテストされています。境界値（1ページ目と2ページ目の境界）も含めて検証されており、十分なカバレッジです。app/models/user.rb (2)
956-958: LGTM!
ordered_micro_reportsメソッドはシンプルで明確な実装です。分報の昇順ソートを提供しており、可読性が高いです。
960-966: LGTM!
page_of_micro_reportメソッドのページ計算ロジックが正しく実装されています。micro_report_idが見つからない場合のnil返却も適切に処理されています。test/models/micro_report_test.rb (1)
6-10: LGTM!
MicroReport#pathメソッドのテストが適切に実装されています。期待されるURL形式を正しく検証しています。test/system/user/micro_reports_test.rb (1)
224-235: LGTM!micro_report_idパラメータによるリダイレクト機能のシステムテストが適切に実装されています。PAGER_NUMBER定数を使用して実際のページネーション境界をテストしており、リダイレクト後のページ番号と対象要素の存在を正しく検証しています。
app/models/micro_report.rb (1)
5-12: Mentioner の組み込みパターンが既存実装と整合していて良いです
include Mentionerとmentionable_as :content、alias sender userの組み合わせは、他のメンション対応モデルと同じインターフェースになっていそうで、一貫性もあり問題なさそうです（通知生成側からも扱いやすそうです）。
|
@yokomaru 期限は2週間前後でお願いしたいですが、特に急いでないのでそれ以降でも構いません。 |
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
d31acce to
3b7f580
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/models/user.rb (1)
965-971: ロジックは正しいですが、パフォーマンスについて考慮点があります。
pluck(:id)はすべての ID をメモリにロードするため、分報の件数が非常に多いユーザーの場合にパフォーマンスに影響する可能性があります。ただし、分報はユーザー単位でスコープされているため、通常の利用では問題ないレベルと考えられます。将来的に件数が増加した場合は、SQL ベースのアプローチ（例:
ROW_NUMBER()ウィンドウ関数）への変更を検討してください。app/controllers/users/micro_reports_controller.rb (1)
11-16: リダイレクトロジックは適切に実装されています。通知クリック時の遷移処理として、適切なページとアンカーへのリダイレクトが実装されています。
@user.micro_reports.findにより、他ユーザーの分報へのアクセスも防止されています。1点だけ軽微な改善提案があります：
params[:micro_report_id]の変換を統一することで、より明確になります。🔎 軽微な改善提案
def index if params[:micro_report_id].present? - micro_report = @user.micro_reports.find(params[:micro_report_id]) - page = @user.page_of_micro_report(params[:micro_report_id].to_i, PAGER_NUMBER) + micro_report_id = params[:micro_report_id].to_i + micro_report = @user.micro_reports.find(micro_report_id) + page = @user.page_of_micro_report(micro_report_id, PAGER_NUMBER) return redirect_to user_micro_reports_path(@user, page:, anchor: "micro_report_#{micro_report.id}") end
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/controllers/users/micro_reports_controller.rbapp/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/user.rbdb/fixtures/micro_reports.ymltest/models/micro_report_test.rbtest/models/user_test.rbtest/system/notification/mention_test.rbtest/system/user/micro_reports_test.rb
🚧 Files skipped from review as they are similar to previous changes (4)
- test/system/notification/mention_test.rb
- test/models/micro_report_test.rb
- test/models/user_test.rb
- test/system/user/micro_reports_test.rb
🧰 Additional context used
📓 Path-based instructions (3)
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/user.rbapp/controllers/users/micro_reports_controller.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
app/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/user.rbapp/controllers/users/micro_reports_controller.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/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/user.rbapp/controllers/users/micro_reports_controller.rb
db/**
📄 CodeRabbit inference engine (AGENTS.md)
Database migrations and schema should be stored in
db/directory; app-specific utilities inlib/; static assets inpublic/
Files:
db/fixtures/micro_reports.yml
🧠 Learnings (3)
📓 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: 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/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: reckyy
Repo: fjordllc/bootcamp PR: 8356
File: app/models/notification.rb:45-47
Timestamp: 2025-07-15T12:58:02.735Z
Learning: fjordllc/bootcampプロジェクトの`app/models/notification.rb`において、`training_completed`通知種別は`TARGETS_TO_KINDS`ハッシュに含めない設計になっている。これは`training_completed`が通知一覧のフィルタ対象ではないためで、意図的な設計判断である。
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: 2025-11-19T07:55:51.504Z
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 名前空間に新しいルートを追加しない方針である。
Applied to files:
app/models/concerns/mentioner.rbapp/models/micro_report.rbdb/fixtures/micro_reports.ymlapp/models/user.rbapp/controllers/users/micro_reports_controller.rb
📚 Learning: 2025-07-23T20:31:13.856Z
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'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Applied to files:
app/controllers/users/micro_reports_controller.rb
🧬 Code graph analysis (2)
app/models/user.rb (1)
app/controllers/users/micro_reports_controller.rb (1)
index(10-19)
app/controllers/users/micro_reports_controller.rb (1)
app/models/user.rb (2)
page_of_micro_report(965-971)ordered_micro_reports(961-963)
⏰ 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 (6)
app/models/concerns/mentioner.rb (1)
27-28: LGTM!
MicroReport用のブランチが既存のパターンに沿って適切に追加されています。分報にはタイトルがないため、シンプルな形式で正しいです。db/fixtures/micro_reports.yml (1)
32-37: LGTM!ページネーションと分報へのリダイレクト機能のテストに必要なフィクスチャが適切に追加されています。
comment_userの省略はset_default_comment_userコールバックによりuserがデフォルト設定されるため問題ありません。app/models/user.rb (1)
961-963: LGTM!シンプルで明確な実装です。コントローラーから参照され、一貫した並び順を提供しています。
app/models/micro_report.rb (2)
15-20:pathメソッドの実装は意図通りです。
micro_report_idをクエリパラメータとして渡すことで、コントローラーのindexアクションが適切なページとアンカーにリダイレクトできます。分報には専用の show ページがないため、この設計は PR の目的に合致しています。
5-5: Mentionerの統合は適切に実装されています。
mentionable_as :contentとalias sender userの組み合わせにより、Mentionerがメンション機能を正しく実装するために必要なすべてのインターフェースが提供されています。mentionable_asマクロはmentionable gem（~> 0.3.0）によって提供され、メンション検出と自動コールバックを設定します。app/controllers/users/micro_reports_controller.rb (1)
18-18: LGTM!
ordered_micro_reportsを使用することで、ページネーション計算と一貫した並び順が保証されています。
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
3b7f580 to
0ad5f17
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/models/user.rb (1)
965-971: 分報数が多い場合のパフォーマンスを考慮できます現在の実装は
pluck(:id)ですべての分報IDをメモリに読み込みます。通常の利用では問題ありませんが、ユーザーが数千件の分報を持つ場合、パフォーマンスに影響する可能性があります。🔎 SQL COUNT を使用した代替実装案
より効率的なSQL実装の例:
def page_of_micro_report(micro_report_id, per_page) target = ordered_micro_reports.find_by(id: micro_report_id) return nil unless target # Count records before the target micro_report count = ordered_micro_reports.where('created_at < ? OR (created_at = ? AND id <= ?)', target.created_at, target.created_at, micro_report_id).count (count - 1) / per_page + 1 endただし、現在の実装でも大多数のユースケースでは十分機能するため、実際のパフォーマンス問題が発生しない限り変更は不要です。
app/controllers/users/micro_reports_controller.rb (1)
11-16: メンション通知からの遷移ロジックが適切に実装されています
@user.micro_reports.find()により、該当する分報がユーザーに属することを確認してから、正しいページとアンカーへリダイレクトしています。セキュリティ面でも問題ありません。💡 オプション: micro_reportレコード全体の読み込みを避ける方法
現在はLine 12で分報レコード全体を読み込んでいますが、アンカー用にはIDのみ使用しています。存在確認のみが目的であれば、以下のように簡略化できます:
if params[:micro_report_id].present? micro_report_id = params[:micro_report_id].to_i # 存在確認のみ（レコードを読み込まない） unless @user.micro_reports.exists?(micro_report_id) raise ActiveRecord::RecordNotFound end page = @user.page_of_micro_report(micro_report_id, PAGER_NUMBER) return redirect_to user_micro_reports_path(@user, page:, anchor: "micro_report_#{micro_report_id}") endただし、現在の実装も十分明確で読みやすいため、変更は任意です。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/controllers/users/micro_reports_controller.rbapp/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/user.rbdb/fixtures/micro_reports.ymltest/models/micro_report_test.rbtest/models/user_test.rbtest/system/notification/mention_test.rbtest/system/user/micro_reports_test.rb
🚧 Files skipped from review as they are similar to previous changes (7)
- test/models/micro_report_test.rb
- db/fixtures/micro_reports.yml
- test/system/notification/mention_test.rb
- test/system/user/micro_reports_test.rb
- app/models/micro_report.rb
- app/models/concerns/mentioner.rb
- test/models/user_test.rb
🧰 Additional context used
📓 Path-based instructions (2)
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/models/user.rbapp/controllers/users/micro_reports_controller.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
app/models/user.rbapp/controllers/users/micro_reports_controller.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/models/user.rbapp/controllers/users/micro_reports_controller.rb
🧠 Learnings (3)
📓 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: 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/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: reckyy
Repo: fjordllc/bootcamp PR: 8356
File: app/models/notification.rb:45-47
Timestamp: 2025-07-15T12:58:02.735Z
Learning: fjordllc/bootcampプロジェクトの`app/models/notification.rb`において、`training_completed`通知種別は`TARGETS_TO_KINDS`ハッシュに含めない設計になっている。これは`training_completed`が通知一覧のフィルタ対象ではないためで、意図的な設計判断である。
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9323
File: app/models/question_auto_closer.rb:45-51
Timestamp: 2025-11-25T05:42:10.478Z
Learning: fjordllc/bootcamp の QuestionAutoCloser において、create_warning_message で発行される 'answer.create' イベントは、システムユーザーによる自動クローズ警告メッセージを質問者と既存の監視者に通知するための意図的な設計である。AnswerNotifier と NotifierToWatchingUser による通知は期待される動作。
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: 2025-11-19T07:55:51.504Z
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 名前空間に新しいルートを追加しない方針である。
Applied to files:
app/models/user.rbapp/controllers/users/micro_reports_controller.rb
📚 Learning: 2025-07-23T20:31:13.856Z
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'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Applied to files:
app/controllers/users/micro_reports_controller.rb
🧬 Code graph analysis (2)
app/models/user.rb (1)
app/controllers/users/micro_reports_controller.rb (1)
index(10-19)
app/controllers/users/micro_reports_controller.rb (1)
app/models/user.rb (2)
page_of_micro_report(965-971)ordered_micro_reports(961-963)
⏰ 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)
app/models/user.rb (1)
961-963: LGTM!分報の順序をモデル側でカプセル化する良いリファクタリングです。コントローラーの可読性が向上し、このクエリロジックを他の場所でも再利用できるようになります。
app/controllers/users/micro_reports_controller.rb (1)
18-18: LGTM!
ordered_micro_reportsメソッドの使用により、コントローラーがシンプルになり、クエリロジックがモデルにカプセル化されています。
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
0ad5f17 to
f0ee5c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/models/user.rb (1)
965-971: 大量データ時のパフォーマンスを考慮してください。
pluck(:id)で全IDをメモリに読み込むため、micro_reportsが数千件を超える場合にパフォーマンスの問題が発生する可能性があります。ただし、分報の性質上、極端に大量のデータになるケースは限定的と考えられるため、現状の実装でも許容範囲内かもしれません。将来的にスケーラビリティが課題になった場合は、SQL内で位置を計算するアプローチをご検討ください。
🔎 SQLベースの代替実装案
def page_of_micro_report(micro_report_id, per_page) result = ordered_micro_reports .select('COUNT(*) as position') .where('micro_reports.id <= ?', micro_report_id) .take return nil unless result && ordered_micro_reports.exists?(micro_report_id) (result.position - 1) / per_page + 1 endただし、現在の実装のシンプルさとのトレードオフを考慮して判断してください。
app/controllers/users/micro_reports_controller.rb (1)
11-16: ページ計算結果がnilの場合のハンドリングを確認してください。
page_of_micro_reportがnilを返す可能性があります（Line 13）。通常はLine 12でActiveRecord::RecordNotFoundが発生するため問題ありませんが、データの不整合がある場合に備えて、明示的なnilチェックを追加することをご検討ください。🔎 nilチェックを追加する場合の実装例
if params[:micro_report_id].present? micro_report = @user.micro_reports.find(params[:micro_report_id]) page = @user.page_of_micro_report(params[:micro_report_id].to_i, PAGER_NUMBER) + + unless page + redirect_to user_micro_reports_path(@user), alert: '指定された分報が見つかりませんでした。' + return + end return redirect_to user_micro_reports_path(@user, page:, anchor: "micro_report_#{micro_report.id}") end
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/controllers/users/micro_reports_controller.rbapp/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/user.rbdb/fixtures/micro_reports.ymltest/models/micro_report_test.rbtest/models/user_test.rbtest/supports/notification_helper.rbtest/system/notification/mention_test.rbtest/system/user/micro_reports_test.rb
🚧 Files skipped from review as they are similar to previous changes (5)
- test/models/micro_report_test.rb
- test/system/user/micro_reports_test.rb
- test/models/user_test.rb
- app/models/micro_report.rb
- db/fixtures/micro_reports.yml
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.rb: Test suite should use Minitest with structure:system/,models/,controllers/, and fixtures intest/fixtures/; test files should be named*_test.rb
Use Minitest + Capybara for system tests; place unit and integration tests under matchingtest/*directories
Files:
test/system/notification/mention_test.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
test/system/notification/mention_test.rbapp/models/concerns/mentioner.rbapp/models/user.rbtest/supports/notification_helper.rbapp/controllers/users/micro_reports_controller.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/notification/mention_test.rbapp/models/concerns/mentioner.rbapp/models/user.rbtest/supports/notification_helper.rbapp/controllers/users/micro_reports_controller.rb
test/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep tests deterministic and use fixtures stored in
test/fixtures/for test data
Files:
test/system/notification/mention_test.rbtest/supports/notification_helper.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/notification/mention_test.rbtest/supports/notification_helper.rb
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/models/concerns/mentioner.rbapp/models/user.rbapp/controllers/users/micro_reports_controller.rb
🧠 Learnings (4)
📓 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: 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/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
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: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Use Minitest + Capybara for system tests; place unit and integration tests under matching `test/*` directories
Applied to files:
test/system/notification/mention_test.rb
📚 Learning: 2025-11-19T07:55:51.504Z
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 名前空間に新しいルートを追加しない方針である。
Applied to files:
test/system/notification/mention_test.rbapp/models/concerns/mentioner.rbapp/models/user.rbapp/controllers/users/micro_reports_controller.rb
📚 Learning: 2025-07-23T20:31:13.856Z
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'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Applied to files:
app/controllers/users/micro_reports_controller.rb
🧬 Code graph analysis (3)
test/system/notification/mention_test.rb (2)
app/components/users/micro_reports/micro_report_component.rb (1)
comment_user(10-12)test/supports/notification_helper.rb (1)
notification_message_for_mention(70-72)
app/models/user.rb (1)
app/controllers/users/micro_reports_controller.rb (1)
index(10-19)
app/controllers/users/micro_reports_controller.rb (1)
app/models/user.rb (2)
page_of_micro_report(965-971)ordered_micro_reports(961-963)
⏰ 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 (5)
app/models/concerns/mentioner.rb (1)
27-28: LGTM!MicroReportに対するメンション処理が既存のパターンに沿って正しく実装されています。
app/models/user.rb (1)
961-963: LGTM!シンプルで明確な実装です。コントローラーで使用される順序付きmicro_reportsを提供しています。
app/controllers/users/micro_reports_controller.rb (1)
18-18: LGTM!
ordered_micro_reportsメソッドの使用により、コードがより読みやすく保守しやすくなっています。test/system/notification/mention_test.rb (1)
80-95: LGTM!分報からのメンション機能のシステムテストが適切に実装されています。既存のテストパターンに沿っており、主要な動作フローをカバーしています。
test/supports/notification_helper.rb (1)
70-72: LGTM!分報のメンション通知メッセージをフォーマットするヘルパーメソッドが適切に実装されています。既存のヘルパーパターンに従っており、テストコードで再利用可能です。
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
f0ee5c5 to
5bd4b9c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/models/user.rb (1)
965-971: パフォーマンスの最適化を検討できます（オプション）。ロジックは正しく動作しますが、
pluck(:id)で全てのIDをメモリに読み込むため、分報の数が非常に多い場合（例：数千件以上）にパフォーマンスへの影響が考えられます。ただし、通常の使用パターンではこの実装で問題ないと思われます。将来的にパフォーマンス問題が発生した場合は、SQLベースのページ計算（COUNT と OFFSET を使用）への変更を検討できます。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/controllers/users/micro_reports_controller.rbapp/models/micro_report.rbapp/models/micro_report_callbacks.rbapp/models/user.rbtest/models/micro_report_test.rbtest/models/user_test.rbtest/system/user/micro_reports_test.rb
🚧 Files skipped from review as they are similar to previous changes (5)
- test/models/user_test.rb
- test/system/user/micro_reports_test.rb
- app/models/micro_report.rb
- app/controllers/users/micro_reports_controller.rb
- test/models/micro_report_test.rb
🧰 Additional context used
📓 Path-based instructions (2)
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/models/user.rbapp/models/micro_report_callbacks.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
app/models/user.rbapp/models/micro_report_callbacks.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/models/user.rbapp/models/micro_report_callbacks.rb
🧠 Learnings (2)
📓 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: 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/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
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'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Learnt from: matuaya
Repo: fjordllc/bootcamp PR: 9108
File: app/models/after_user_retirement.rb:0-0
Timestamp: 2025-09-01T22:31:57.345Z
Learning: fjordllc/bootcampプロジェクトにおいて、`Newspaper.publish(:retirement_create, { user: user })`はUnfinishedDataDestroyer（未チェックProduct・WIP Report削除、career_pathリセット）とTimesChannelDestroyer（Discord分報チャンネル削除）による重要なデータクリーンアップ処理を実行するため、退会のトリガーが何であろうと（admin、hibernation、user）必ず実行する必要がある。
Learnt from: matuaya
Repo: fjordllc/bootcamp PR: 9108
File: app/models/after_user_retirement.rb:0-0
Timestamp: 2025-09-01T22:31:57.345Z
Learning: fjordllc/bootcampプロジェクトにおいて、`Newspaper.publish(:retirement_create, { user: user })`は通知処理ではなく、UnfinishedDataDestroyerやTimesChannelDestroyer等による重要なデータクリーンアップ処理を実行するため、管理者による退会（triggered_by: 'admin'）の場合でも必ず実行する必要がある。
📚 Learning: 2025-11-19T07:55:51.504Z
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 名前空間に新しいルートを追加しない方針である。
Applied to files:
app/models/user.rbapp/models/micro_report_callbacks.rb
🧬 Code graph analysis (2)
app/models/user.rb (1)
app/controllers/users/micro_reports_controller.rb (1)
index(10-25)
app/models/micro_report_callbacks.rb (1)
app/models/micro_report.rb (1)
path(17-22)
⏰ 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)
app/models/user.rb (1)
961-963: 実装が適切です。分報を作成日時の昇順で取得するメソッドで、コントローラーでのページネーション処理と一貫性があります。命名も明確で分かりやすいです。
app/models/micro_report_callbacks.rb (1)
1-13: 実装が適切です。分報削除時に関連する通知を削除するコールバッククラスで、実装は明確です。
destroy_allを使用することでコールバックが実行され、micro_report.pathをキーに通知を検索する仕組みも適切です。
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
5bd4b9c to
a43e565
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/models/user.rb (1)
965-971:per_pageパラメータの妥当性検証を検討してください
per_pageが0または負の値の場合、ゼロ除算または予期しない動作が発生する可能性があります。コントローラー側で定数PAGER_NUMBERを使用しているため現在は問題になりませんが、このメソッドがパブリックAPIとして他の場所から呼び出される可能性を考慮すると、防御的なバリデーションを追加することを推奨します。🔎 提案される修正
def page_of_micro_report(micro_report_id, per_page) + return nil if per_page <= 0 + ids = ordered_micro_reports.pluck(:id) index = ids.index(micro_report_id) return nil unless index index / per_page + 1 endapp/models/micro_report_callbacks.rb (1)
10-11:destroy_allとdelete_allのパフォーマンスを検討してください
destroy_allは各通知のコールバックを実行しますが、Notificationモデルに削除時の重要なコールバックがない場合、delete_allを使用することでパフォーマンスが向上します。メンション通知の数が限定的であれば現在の実装でも問題ありませんが、将来的な最適化の余地として検討する価値があります。🔎 delete_all を使用する場合の実装例
def delete_notification(micro_report) - Notification.where(link: micro_report.path).destroy_all + Notification.where(link: micro_report.path).delete_all end注意:
Notificationモデルに削除時の重要なコールバック（dependent: :destroy な関連など）がある場合は、現在のdestroy_allを維持してください。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/controllers/users/micro_reports_controller.rbapp/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/micro_report_callbacks.rbapp/models/user.rbdb/fixtures/micro_reports.ymltest/models/micro_report_test.rbtest/models/user_test.rbtest/supports/notification_helper.rbtest/system/notification/mention_test.rbtest/system/user/micro_reports_test.rb
🚧 Files skipped from review as they are similar to previous changes (5)
- app/controllers/users/micro_reports_controller.rb
- app/models/concerns/mentioner.rb
- app/models/micro_report.rb
- test/models/micro_report_test.rb
- test/models/user_test.rb
🧰 Additional context used
📓 Path-based instructions (6)
db/**
📄 CodeRabbit inference engine (AGENTS.md)
Database migrations and schema should be stored in
db/directory; app-specific utilities inlib/; static assets inpublic/
Files:
db/fixtures/micro_reports.yml
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/models/micro_report_callbacks.rbapp/models/user.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
app/models/micro_report_callbacks.rbtest/supports/notification_helper.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rbapp/models/user.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/models/micro_report_callbacks.rbtest/supports/notification_helper.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rbapp/models/user.rb
test/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep tests deterministic and use fixtures stored in
test/fixtures/for test data
Files:
test/supports/notification_helper.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/supports/notification_helper.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.rb: Test suite should use Minitest with structure:system/,models/,controllers/, and fixtures intest/fixtures/; test files should be named*_test.rb
Use Minitest + Capybara for system tests; place unit and integration tests under matchingtest/*directories
Files:
test/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
🧠 Learnings (5)
📓 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: 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/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: reckyy
Repo: fjordllc/bootcamp PR: 8356
File: app/models/notification.rb:45-47
Timestamp: 2025-07-15T12:58:02.735Z
Learning: fjordllc/bootcampプロジェクトの`app/models/notification.rb`において、`training_completed`通知種別は`TARGETS_TO_KINDS`ハッシュに含めない設計になっている。これは`training_completed`が通知一覧のフィルタ対象ではないためで、意図的な設計判断である。
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9323
File: app/models/question_auto_closer.rb:45-51
Timestamp: 2025-11-25T05:42:10.478Z
Learning: fjordllc/bootcamp の QuestionAutoCloser において、create_warning_message で発行される 'answer.create' イベントは、システムユーザーによる自動クローズ警告メッセージを質問者と既存の監視者に通知するための意図的な設計である。AnswerNotifier と NotifierToWatchingUser による通知は期待される動作。
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'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Learnt from: matuaya
Repo: fjordllc/bootcamp PR: 9108
File: app/models/after_user_retirement.rb:0-0
Timestamp: 2025-09-01T22:31:57.345Z
Learning: fjordllc/bootcampプロジェクトにおいて、`Newspaper.publish(:retirement_create, { user: user })`はUnfinishedDataDestroyer（未チェックProduct・WIP Report削除、career_pathリセット）とTimesChannelDestroyer（Discord分報チャンネル削除）による重要なデータクリーンアップ処理を実行するため、退会のトリガーが何であろうと（admin、hibernation、user）必ず実行する必要がある。
📚 Learning: 2025-11-19T07:55:51.504Z
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 名前空間に新しいルートを追加しない方針である。
Applied to files:
db/fixtures/micro_reports.ymlapp/models/micro_report_callbacks.rbtest/system/user/micro_reports_test.rbtest/system/notification/mention_test.rbapp/models/user.rb
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Use Minitest + Capybara for system tests; place unit and integration tests under matching `test/*` directories
Applied to files:
test/system/user/micro_reports_test.rbtest/system/notification/mention_test.rb
📚 Learning: 2025-07-10T12:59:27.807Z
Learnt from: kitarou888
Repo: fjordllc/bootcamp PR: 8277
File: test/models/searcher_test.rb:326-331
Timestamp: 2025-07-10T12:59:27.807Z
Learning: モデルテストにおいて、1つのクラスメソッドの複数の挙動を検証する場合、機能の異なる側面を同じテストメソッドでテストすることは、包括的なテストとして適切である。特に`only_me`のような機能では、異なる検索条件（空文字と具体的な検索語）を使い分けることで、より広範囲な動作保証が可能となる。
Applied to files:
test/system/user/micro_reports_test.rb
📚 Learning: 2025-07-30T07:26:36.599Z
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: Railsの標準的なヘルパーメソッド（例：`question_url`、`user_path`など）が通常の方法で使用されている場合、それらの動作テストは不要である。これらのメソッドはRailsフレームワーク自体でテストされており、アプリケーション固有のロジックではないため。
Applied to files:
test/system/user/micro_reports_test.rb
🧬 Code graph analysis (3)
app/models/micro_report_callbacks.rb (1)
app/models/micro_report.rb (1)
path(18-23)
test/system/notification/mention_test.rb (2)
app/components/users/micro_reports/micro_report_component.rb (1)
comment_user(10-12)test/supports/notification_helper.rb (1)
notification_message_for_mention(70-72)
app/models/user.rb (1)
app/controllers/users/micro_reports_controller.rb (1)
index(10-25)
⏰ 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 (8)
db/fixtures/micro_reports.yml (1)
32-37: LGTM!ページネーションのテストに必要な十分なフィクスチャデータが追加されています。
app/models/user.rb (2)
961-963: LGTM!分報を作成日時の昇順で取得するシンプルで明確な実装です。
961-971: ユニットテストは既に実装済みです
test/models/user_test.rbにおいて、両メソッドのテストが既に追加されています：
#ordered_micro_reports（776-787行）：created_at による昇順ソートを検証#page_of_micro_report（789-803行）：複数レポートのページネーション計算を検証test/system/notification/mention_test.rb (1)
80-95: LGTM!分報のメンション機能のシステムテストが適切に実装されています。既存のメンションテストのパターンに従い、投稿から通知確認までのエンドツーエンドのフローをカバーしています。
test/supports/notification_helper.rb (1)
70-72: LGTM!分報のメンション通知メッセージをフォーマットするヘルパーメソッドが適切に実装されています。既存のヘルパーメソッドのパターンに従い、テストコードの可読性を向上させています。
test/system/user/micro_reports_test.rb (2)
236-247: LGTM!
micro_report_idパラメータを使用した特定の分報へのリダイレクト機能が適切にテストされています。ページ番号の計算とアンカーへの遷移を正しく検証しています。
249-256: LGTM!存在しない
micro_report_idに対するエラーハンドリングが適切にテストされています。アラートメッセージの表示とフォールバック動作を正しく検証しています。app/models/micro_report_callbacks.rb (1)
3-6: コールバックパターンの実装が適切ですPORを使用したコールバックオブジェクトパターンにより、MicroReportモデルをスリムに保ちつつ、削除時の処理を明確に分離できています。コーディングガイドラインに沿った設計です。
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
a43e565 to
42b9be9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/controllers/users/micro_reports_controller.rb (1)
20-21: 既存の変数を使用して冗長性を削減Line 20で
params[:micro_report_id].to_iを使用していますが、Line 12で既にmicro_report変数を取得しています。一貫性と可読性のため、micro_report.idを使用することを推奨します。🔎 提案される修正
- page = @user.page_of_micro_report(params[:micro_report_id].to_i, PAGER_NUMBER) + page = @user.page_of_micro_report(micro_report.id, PAGER_NUMBER) return redirect_to user_micro_reports_path(@user, page:, anchor: "micro_report_#{micro_report.id}")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/controllers/users/micro_reports_controller.rbapp/models/concerns/mentioner.rbapp/models/micro_report.rbapp/models/micro_report_callbacks.rbapp/models/user.rbapp/views/activity_mailer/mentioned.html.slimdb/fixtures/micro_reports.ymltest/models/micro_report_test.rbtest/models/user_test.rbtest/supports/notification_helper.rbtest/system/notification/mention_test.rbtest/system/user/micro_reports_test.rb
🚧 Files skipped from review as they are similar to previous changes (7)
- app/models/concerns/mentioner.rb
- app/models/micro_report.rb
- test/models/micro_report_test.rb
- app/models/user.rb
- db/fixtures/micro_reports.yml
- test/system/notification/mention_test.rb
- test/models/user_test.rb
🧰 Additional context used
📓 Path-based instructions (6)
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/controllers/users/micro_reports_controller.rbapp/models/micro_report_callbacks.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
app/controllers/users/micro_reports_controller.rbapp/models/micro_report_callbacks.rbtest/supports/notification_helper.rbtest/system/user/micro_reports_test.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/users/micro_reports_controller.rbapp/models/micro_report_callbacks.rbtest/supports/notification_helper.rbtest/system/user/micro_reports_test.rb
**/*.slim
📄 CodeRabbit inference engine (AGENTS.md)
Slim templates should be linted according to
config/slim_lint.yml
Files:
app/views/activity_mailer/mentioned.html.slim
test/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep tests deterministic and use fixtures stored in
test/fixtures/for test data
Files:
test/supports/notification_helper.rbtest/system/user/micro_reports_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/supports/notification_helper.rbtest/system/user/micro_reports_test.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.rb: Test suite should use Minitest with structure:system/,models/,controllers/, and fixtures intest/fixtures/; test files should be named*_test.rb
Use Minitest + Capybara for system tests; place unit and integration tests under matchingtest/*directories
Files:
test/system/user/micro_reports_test.rb
🧠 Learnings (7)
📓 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: 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/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: reckyy
Repo: fjordllc/bootcamp PR: 8356
File: app/models/notification.rb:45-47
Timestamp: 2025-07-15T12:58:02.735Z
Learning: fjordllc/bootcampプロジェクトの`app/models/notification.rb`において、`training_completed`通知種別は`TARGETS_TO_KINDS`ハッシュに含めない設計になっている。これは`training_completed`が通知一覧のフィルタ対象ではないためで、意図的な設計判断である。
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9323
File: app/models/question_auto_closer.rb:45-51
Timestamp: 2025-11-25T05:42:10.478Z
Learning: fjordllc/bootcamp の QuestionAutoCloser において、create_warning_message で発行される 'answer.create' イベントは、システムユーザーによる自動クローズ警告メッセージを質問者と既存の監視者に通知するための意図的な設計である。AnswerNotifier と NotifierToWatchingUser による通知は期待される動作。
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: 2025-11-19T07:55:51.504Z
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 名前空間に新しいルートを追加しない方針である。
Applied to files:
app/controllers/users/micro_reports_controller.rbapp/models/micro_report_callbacks.rbtest/system/user/micro_reports_test.rb
📚 Learning: 2025-07-23T20:31:13.856Z
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'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Applied to files:
app/controllers/users/micro_reports_controller.rb
📚 Learning: 2025-07-23T21:11:21.826Z
Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 0
File: :0-0
Timestamp: 2025-07-23T21:11:21.826Z
Learning: fjordllc/bootcampプロジェクトの`app/models/inquiry_notifier.rb`において、`NOTIFICATION_SENDER_NAME`で指定される管理者ユーザーは必ず存在することがシステムの前提条件として保証されているため、senderがnilになることは想定されておらず、エラーハンドリングは不要である。
Applied to files:
app/views/activity_mailer/mentioned.html.slim
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Use Minitest + Capybara for system tests; place unit and integration tests under matching `test/*` directories
Applied to files:
test/system/user/micro_reports_test.rb
📚 Learning: 2025-07-10T12:59:27.807Z
Learnt from: kitarou888
Repo: fjordllc/bootcamp PR: 8277
File: test/models/searcher_test.rb:326-331
Timestamp: 2025-07-10T12:59:27.807Z
Learning: モデルテストにおいて、1つのクラスメソッドの複数の挙動を検証する場合、機能の異なる側面を同じテストメソッドでテストすることは、包括的なテストとして適切である。特に`only_me`のような機能では、異なる検索条件（空文字と具体的な検索語）を使い分けることで、より広範囲な動作保証が可能となる。
Applied to files:
test/system/user/micro_reports_test.rb
📚 Learning: 2025-07-30T07:26:36.599Z
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: Railsの標準的なヘルパーメソッド（例：`question_url`、`user_path`など）が通常の方法で使用されている場合、それらの動作テストは不要である。これらのメソッドはRailsフレームワーク自体でテストされており、アプリケーション固有のロジックではないため。
Applied to files:
test/system/user/micro_reports_test.rb
🧬 Code graph analysis (2)
app/controllers/users/micro_reports_controller.rb (1)
app/models/user.rb (2)
page_of_micro_report(965-971)ordered_micro_reports(961-963)
app/models/micro_report_callbacks.rb (1)
app/models/micro_report.rb (1)
path(18-23)
⏰ 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 (6)
app/controllers/users/micro_reports_controller.rb (1)
24-24: LGTM!
ordered_micro_reportsメソッドを使用することで、ソート処理をモデルに委譲し、コントローラーの責務を適切に分離できています。test/supports/notification_helper.rb (1)
70-72: LGTM!分報のメンション通知メッセージを生成するヘルパーメソッドの実装が適切です。既存のヘルパーメソッドのパターンに従っており、privateセクションに正しく配置されています。
test/system/user/micro_reports_test.rb (2)
236-247: LGTM!
micro_report_idパラメータを指定した場合の対象分報ページへのリダイレクト動作を適切にテストしています。ページ番号とアンカーの両方を検証しており、実装が正しいことを確認できます。
249-256: LGTM!存在しない
micro_report_idを指定した場合のエラーハンドリングを適切にテストしています。アラートメッセージの検証も含まれており、ユーザーへのフィードバックが正しく機能することを確認できます。app/models/micro_report_callbacks.rb (1)
1-13: LGTM!MicroReportの削除時に関連する通知を自動的にクリーンアップするコールバック実装が適切です。
micro_report.pathを使用して関連通知を特定する設計は、他のモデルのパターンと一貫性があります。app/views/activity_mailer/mentioned.html.slim (1)
2-2: この変更は安全です - すべてのmentionableタイプが既にsenderメソッドを定義していますReport、Comment、Question、Answer、Product、Page、Announcement、CheckおよびArticleを含むすべてのMentionerを含むモデルに対して、既に
senderエイリアスが定義されています。MicroReportはcomment_userを、その他のモデルはuserをsenderとして定義しており、@mentionable.user.login_nameから@mentionable.sender.login_nameへの変更は既存の実装と一貫性を持ち、NoMethodErrorは発生しません。Likely an incorrect or invalid review comment.
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
42b9be9 to
345e5fd
Compare
|
@tyrrell-IH |
|
@yokomaru 3週間〜とかでも全然大丈夫ですのでよろしくお願いします〜 |
|
@tyrrell-IH |
|
ご連絡ありがとうございます🙏 |
yokomaru
left a comment
yokomaru
left a comment
There was a problem hiding this comment.
@tyrrell-IH
お疲れ様です！レビューを返すのが遅くなってしまい大変申し訳ありません！🙏
動作確認は問題なく、実装やテストも丁寧に書かれていて勉強になりました！
手順の動線も分かりやすくとても検証しやすかったです👍👍
1点だけ意図をお伺いしたい箇所があり、そちらにコメントしていますのでご確認いただけますと幸いです🙌
| def ordered_micro_reports | ||
| micro_reports.order(created_at: :asc) | ||
| end | ||
|
|
||
| def page_of_micro_report(micro_report_id, per_page) | ||
| return nil if per_page <= 0 | ||
|
|
||
| ids = ordered_micro_reports.pluck(:id) | ||
| index = ids.index(micro_report_id) | ||
| return nil unless index | ||
|
|
||
| index / per_page + 1 | ||
| end |
There was a problem hiding this comment.
ordered_micro_reportsとpage_of_micro_reportをUserモデル側に置いている意図について確認させてください！💡
並び順や自身のページを返すような「分報自身(分報のまとまり)を扱うロジック」はUserが持つよりも、MicroReport側にある方がモデルとして責務が分離できるのかな？と思いました！
以下のような感じをイメージしております。
- 並び順は
MicroReport側にscopeとして切り出す - ページは
MicroReportに分報のまとまりと対象IDとper_pageを渡して何ページ目かを返してもらう
ただuser/:id/micro_reportsというリソースを操作している観点では、現状の置き場所でも不自然でないとも感じていて判断が難しい部分だなと思っており、今回Userモデル側に定義いただいた意図や背景があればぜひお伺いできたらと思いました！🙌
処理自体はindexメソッドからインデックスを取得するところなど、とてもすっきり書けていて勉強になりました！✨
There was a problem hiding this comment.
今回Userモデル側に定義いただいた意図や背景があればぜひお伺いできたらと思いました！🙌
micro_reports_controller.rb内で@user.micro_reportsのようにユーザーの関連で分報を引く書き方が多かったので、Userクラスのインスタンスを起点に分報の並びを考える方がいいのかなと思って@user.並び替えられた分報を返すメソッドの形でずっと考えてました。
責務をUserに置くかMicroReportに置くかの観点をあまり深く考えていませんでした😓
並び順や自身のページを返すような「分報自身(分報のまとまり)を扱うロジック」はUserが持つよりも、
MicroReport側にある方がモデルとして責務が分離できるのかな？と思いました！
この指摘はその通りだと思ったので、MicroReportモデルクラスのクラスメソッドpage_number_forに作り直しました。
基本的な構成は
並び順はMicroReport側にscopeとして切り出す
ページはMicroReportに分報のまとまりと対象IDとper_pageを渡して何ページ目かを返してもらう
を参考にさせていただきました。
なお、以下2点page_number_for内のちょっとした変更があります
per_page <= 0の箇所で数値以外と比較してエラーが出ないようにper_page = per_page.to_iの行を追加- キーワード引数の方がメソッドの内容を理解しやすいと思ってキーワード引数にしました。
が、過剰であれば普通の引数に戻します。https://bootcamp.fjord.jp/articles/134
There was a problem hiding this comment.
修正ありがとうございます！
メソッド名もMicroReport を起点とした名称として、何を返してくれるか役割がわかりやすくて流石です・・・！！
以下の対応も堅牢かつわかりやすいコードになっていてとても参考になりました🙌
per_page <= 0の箇所で数値以外と比較してエラーが出ないようにper_page = per_page.to_iの行を追加
キーワード引数の方がメソッドの内容を理解しやすいと思ってキーワード引数にしました
レビューしておきながらこれらの処理はどのモデルに置くのが良いか悩んでいたのですが、
tyrrellさんが丁寧に検討してくださったおかげですごく腑に落ちました！
ご意見を聞けて本当に良かったです！ありがとうございましたー！
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
81e3a86 to
ba25cf1
Compare
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
ba25cf1 to
0da860b
Compare
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
0da860b to
834b2a7
Compare
tyrrell-IH
force-pushed
the
feature/send-notification-when-mentioned-in-micro-report
branch
from
834b2a7 to
b8e0120
Compare
|
遅くなりましたがレビューを受けての修正を行いました！ お時間のある時にご確認ください🙏 |
yokomaru
left a comment
yokomaru
left a comment
There was a problem hiding this comment.
お疲れ様です！修正いただきありがとうございます！
1点コメントしたのですが、ほんの少しコードが短くなるかな？くらいなので修正いただかなくても大丈夫です👍
Approveさせていただきます！
引き続きどうぞよろしくお願いしますー！
| per_page = per_page.to_i | ||
| return nil if per_page <= 0 | ||
|
|
||
| ids = scope.ordered.pluck(:id) |
There was a problem hiding this comment.
[NITS]
主キーをpluckする場合はidsを使うとよりスッキリ書けそうな気がします💡
(コードを見ると結局pluck(:id)のエイリアスのようなので、どっちでも良さそうなので修正はお任せします！)
https://github.com/rails/rails/blob/984c3ef2775781d47efa9f541ce570daa2434a80/activerecord/lib/active_record/relation/calculations.rb#L242
There was a problem hiding this comment.
|
@yokomaru 自分では気づかない観点からのレビューを頂けてすごく勉強になりました🙌 @komagata |
komagata
deleted the
feature/send-notification-when-mentioned-in-micro-report
branch
3 participants
Issue
概要
分報の本文にて自分以外のユーザーにメンションした場合、メンションされたユーザーに通知が飛ぶように実装しました。
基本的な動作は日報やコメント等でメンションしたときと同様です。
分報機能では
micro_reports#showに対するルーティング及び画面が用意されていません。なので
notificationsテーブル)のlinkに該当の分報idをクエリとして保存する(例
/users/{user_id}/micro_reports?micro_report_id={micro_report_id})?page={page_number}を、フラグメントとして#micro_report_{micro_report_id}を付け、該当の分報まで遷移するという実装にしています。
変更確認方法
feature/send-notification-when-mentioned-in-micro-reportをローカルに取り込む。bin/rails db:seedを実行する。↓
本PRもRuby on Rails8.0のmainをrebaseしたのでサーバーの立ち上げは
になります。
動作確認
通知が届くことの確認
kimuraでログインしusers/{kimuraのuser_id}/micro_reportsへ移動する。@machida 削除予定ですという内容で投稿する。
users/{kimuraのuser_id}/micro_reportsの2ページ目にメンション付きの分報が投稿されたことを確認する。machidaでログインし、http://localhost:3000/notifications?status=unreadへ移動。?page=2のクエリが付いている)。#micro_report_{id}のフラグメントが付いている)。分報がどのページにあるか計算するロジックが正しく動作しているかの確認
machidaでログインした状態で、kimura所有の「テスト用の分報」を1件削除する。?page=1に変更されている)。#micro_report_{id}のフラグメントが付いている)。通知のメッセージが「（分報所有者）の分報で（分報投稿者）さんからメンションがきました」の形式になっているかの確認
ユーザー
machidaでログインしたまま、ユーザーkimura所有の分報に@kimuraとメンションを入れて投稿。kimuraで再度ログインし、http://localhost:3000/notifications?status=unreadへ移動。参考: メール通知は以下のようになります。
メンション付き分報が削除された場合に、通知も削除
kimuraでログインした状態で、「通知が届くことの確認」で作成した「@machida 削除予定です」という内容の分報を削除する。machidaでログイン、http://localhost:3000/notificationsへ移動する。メンション付き分報が削除された場合の通知の削除について
日報の動作に倣って、メンション付き分報が削除された場合には通知も削除されるように実装しました。
参考: app/models/report_callbacks.rb
削除された分報のlinkを踏んでしまったら
分報のlinkを便宜的に
/users/{user_id}/micro_reports?micro_report_id={micro_report_id}としてnotificationsテーブルに保存しています。メンション付き分報が削除された場合には通知も削除されるので、上記linkも基本的に踏むことはないと考えていますが、もし何らかの理由で踏んでしまった場合には
micro_report#indexの画面を表示するように設定しています。Screenshot
変更前
変更前画像なし
変更後
メンションを送る
サイト内通知
メール通知
URL
Summary by CodeRabbit
新機能
改善 / バグ修正
テスト
✏️ Tip: You can customize this high-level summary in your review settings.