Conversation
WalkthroughQuestionsController が作成・更新時に ActiveSupport::Notifications を発火するように変更し、 Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as QuestionsController
participant ASNoti as ActiveSupport::Notifications
participant MWQ as MentorsWatchForQuestionCreator
participant AIA as AIAnswerCreator
rect rgb(230,245,255)
Controller->>ASNoti: instrument 'question.create' (payload: question)
ASNoti->>MWQ: call(_name,_started,_finished,_unique_id,payload)
ASNoti->>AIA: call(_name,_started,_finished,_unique_id,payload)
end
rect rgb(245,255,230)
Controller->>ASNoti: instrument 'question.update' (payload: question) [if saved_change_to_wip?]
ASNoti->>MWQ: call(_name,_started,_finished,_unique_id,payload)
ASNoti->>AIA: call(_name,_started,_finished,_unique_id,payload)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–30分 Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 RuboCop (1.76.1)app/models/ai_answer_creator.rbrubocop-minitest extension supports plugin, specify config/initializers/active_support_notifications.rbrubocop-minitest extension supports plugin, specify app/controllers/questions_controller.rbrubocop-minitest extension supports plugin, specify
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/controllers/questions_controller.rb(2 hunks)app/models/ai_answer_creator.rb(1 hunks)app/models/mentors_watch_for_question_creator.rb(1 hunks)config/initializers/active_support_notifications.rb(1 hunks)config/initializers/newspaper.rb(0 hunks)
💤 Files with no reviewable changes (1)
- config/initializers/newspaper.rb
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
config/initializers/active_support_notifications.rbapp/models/ai_answer_creator.rbapp/models/mentors_watch_for_question_creator.rbapp/controllers/questions_controller.rb
🧠 Learnings (2)
📓 Common learnings
Learnt from: reckyy
PR: fjordllc/bootcamp#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
PR: fjordllc/bootcamp#0
File: :0-0
Timestamp: 2025-07-23T21:11:21.826Z
Learning: fjordllc/bootcampプロジェクトの`app/models/inquiry_notifier.rb`において、`NOTIFICATION_SENDER_NAME`で指定される管理者ユーザーは必ず存在することがシステムの前提条件として保証されているため、senderがnilになることは想定されておらず、エラーハンドリングは不要である。
app/controllers/questions_controller.rb (1)
Learnt from: jun-kondo
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'イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
⏰ 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 (4)
config/initializers/active_support_notifications.rb (1)
20-23: ActiveSupport::Notificationsへの移行が適切に実装されていますMentorsWatchForQuestionCreatorとAIAnswerCreatorの両方が
question.createとquestion.updateイベントに正しく購読されており、既存の他の通知システムと一貫したパターンに従っています。app/models/ai_answer_creator.rb (1)
4-4: ActiveSupport::Notifications対応のメソッドシグネチャが正しく更新されています未使用パラメータに適切にアンダースコア接頭辞が付けられており、既存のロジックに影響を与えずにActiveSupport::Notificationsのコールバック仕様に準拠しています。
app/models/mentors_watch_for_question_creator.rb (1)
4-4: AIAnswerCreatorと一貫したメソッドシグネチャの更新ActiveSupport::Notificationsのコールバック仕様に準拠した適切なメソッドシグネチャの更新が行われており、AIAnswerCreatorと一貫性が保たれています。
app/controllers/questions_controller.rb (1)
71-73: updateアクションの条件付きイベント発火ロジックは適切です
wip_changedフラグに基づいて両方のイベントシステム（ActiveSupport::NotificationsとNewspaper）で一貫してイベントが発火されており、移行期間中の整合性が保たれています。ただし、変数名の抽出により可読性が向上していることを評価します。
|
@matuaya |
|
@sjabcdefin |
matuaya
left a comment
matuaya
left a comment
There was a problem hiding this comment.
確認しました！
Approveさせていただきます😊
|
@matuaya |
|
@komagata |
|
@sjabcdefin conflictの修正をお願いします。 |
sjabcdefin
force-pushed
the
chore/convert-mentorwatch-and-aianswer-to-notifications
branch
from
b5c7071 to
dec8cd2
Compare
|
@komagata すみません。まだ、CodeRabbit レビュー中でした。 |
|
@komagata よろしくお願いいたします。🙇♀️ |
| wip_changed = @question.saved_change_to_wip? | ||
| ActiveSupport::Notifications.instrument('question.update', { question: @question }) if wip_changed | ||
| Newspaper.publish(:question_update, { question: @question }) if wip_changed |
There was a problem hiding this comment.
後置のifは1行の時の書き方なのでこの場合は下記でいいかもです。
| wip_changed = @question.saved_change_to_wip? | |
| ActiveSupport::Notifications.instrument('question.update', { question: @question }) if wip_changed | |
| Newspaper.publish(:question_update, { question: @question }) if wip_changed | |
| if @question.saved_change_to_wip? | |
| ActiveSupport::Notifications.instrument('question.update', { question: @question }) | |
| Newspaper.publish(:question_update, { question: @question }) | |
| end |
There was a problem hiding this comment.
後置ifを通常のif文に修正いたしました。
コミット：005b9e8
※rebaseの影響で該当のレビューコメントが表示されなくなっているようなので、こちらに返信させていただきました。🙇♀️
| ActiveSupport::Notifications.subscribe('question.create', MentorsWatchForQuestionCreator.new) | ||
| ActiveSupport::Notifications.subscribe('question.update', MentorsWatchForQuestionCreator.new) | ||
| ActiveSupport::Notifications.subscribe('question.create', AIAnswerCreator.new) | ||
| ActiveSupport::Notifications.subscribe('question.update', AIAnswerCreator.new) |
There was a problem hiding this comment.
同じインスタンスが複数作成されることになっちゃうので下記のような感じがいいかもです。
| ActiveSupport::Notifications.subscribe('question.create', MentorsWatchForQuestionCreator.new) | |
| ActiveSupport::Notifications.subscribe('question.update', MentorsWatchForQuestionCreator.new) | |
| ActiveSupport::Notifications.subscribe('question.create', AIAnswerCreator.new) | |
| ActiveSupport::Notifications.subscribe('question.update', AIAnswerCreator.new) | |
| mentors_watch_for_question_creator = MentorsWatchForQuestionCreator.new | |
| ActiveSupport::Notifications.subscribe('question.create', mentors_watch_for_question_creator) | |
| ActiveSupport::Notifications.subscribe('question.update', mentors_watch_for_question_creator) | |
| ai_answer_creator = AIAnswerCreator.new | |
| ActiveSupport::Notifications.subscribe('question.create', ai_answer_creator) | |
| ActiveSupport::Notifications.subscribe('question.update', ai_answer_creator) |
There was a problem hiding this comment.
同一インスタンスを1回だけ作成するように修正いたしました。
コミット：7a08275
※rebaseの影響で該当のレビューコメントが表示されなくなっているようなので、こちらに返信させていただきました。🙇♀️
sjabcdefin
force-pushed
the
chore/convert-mentorwatch-and-aianswer-to-notifications
branch
from
dec8cd2 to
7a08275
Compare
|
@sjabcdefin conflictの修正をお願いします。 |
sjabcdefin
force-pushed
the
chore/convert-mentorwatch-and-aianswer-to-notifications
branch
from
7a08275 to
b5b530c
Compare
sjabcdefin
force-pushed
the
chore/convert-mentorwatch-and-aianswer-to-notifications
branch
3 times, most recently
from
e375235 to
0ce5cd7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/models/mentors_watch_for_question_creator.rb (3)
8-10: insert_all の重複挿入対策（ユニーク制約 + unique_by/upsert）の検討をイベントが何らかの理由で二重発火した場合や並行実行が起きた場合、重複 Watch が挿入され得ます（既存実装でも同様の懸念はありますが、通知方式移行に伴う呼び出しタイミングの違いで顕在化しやすくなる可能性があります）。
- DB に (watchable_type, watchable_id, user_id) のユニークインデックスがあるかご確認ください。
- ある場合は insert_all の unique_by を指定、もしくは upsert_all の利用をご検討ください（PostgreSQL 前提）。
例（ユニークインデックスがある場合）:
Watch.insert_all(watch_question_records, unique_by: :index_watches_on_watchable_and_user) # 例: インデックス名もしくは migration 例（未作成の場合）:
add_index :watches, [:watchable_type, :watchable_id, :user_id], unique: true, name: :index_watches_on_watchable_and_user
12-22: メモリ使用の削減: mentor の id だけを pluck してレコード生成しませんかUser.mentor を AR オブジェクトで全件ロードせず、id だけを pluck すると軽量になります。
適用例:
- def watch_records(question) - User.mentor.map do |mentor| + def watch_records(question) + User.mentor.pluck(:id).map do |mentor_id| { watchable_type: 'Question', watchable_id: question.id, created_at: Time.current, updated_at: Time.current, - user_id: mentor.id + user_id: mentor_id } end end
12-22: 公開インターフェースの最小化: watch_records を private に外部から呼ばれる必要がなければ private 指定が妥当です。クラスの責務と公開APIが明確になります。
def call(_name, _started, _finished, _unique_id, payload) question = payload[:question] return if question.wip? || question.watched? watch_question_records = watch_records(question) Watch.insert_all(watch_question_records) # rubocop:disable Rails/SkipsModelValidations end - def watch_records(question) + private + + def watch_records(question)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
app/controllers/questions_controller.rb(2 hunks)app/models/ai_answer_creator.rb(1 hunks)app/models/mentors_watch_for_question_creator.rb(1 hunks)config/initializers/active_support_notifications.rb(1 hunks)config/initializers/newspaper.rb(0 hunks)
💤 Files with no reviewable changes (1)
- config/initializers/newspaper.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- app/controllers/questions_controller.rb
- app/models/ai_answer_creator.rb
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
config/initializers/active_support_notifications.rbapp/models/mentors_watch_for_question_creator.rb
⏰ 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 (3)
config/initializers/active_support_notifications.rb (2)
45-51: 単一インスタンスを使い回して購読する形に修正されていて良いですKomagataさんの指摘どおり、同一インスタンスを複数イベントへ購読登録しており、不要なインスタンス生成が避けられています。実装方針・書き方ともにこのファイル内の既存パターンとも整合しています。
45-51: to_prepare 内での subscribe は開発環境のリロードで多重購読が積み上がらないか確認を本ファイル全体で採用している既存パターンですが、development でコードがリロードされるたびに subscribe が追加される可能性があります。今回の question.create/update の購読でも同じ挙動になるため、開発環境で質問作成/公開変更時にリスナーの call が複数回実行されていないか、軽く確認をお願いします。
app/models/mentors_watch_for_question_creator.rb (1)
4-5: ActiveSupport::Notifications の5引数シグネチャ対応はOKcall(_name, _started, _finished, _unique_id, payload) に揃っており、未使用引数はプレフィックスで抑制されていて適切です。
|
@komagata |
sjabcdefin
force-pushed
the
chore/convert-mentorwatch-and-aianswer-to-notifications
branch
from
0ce5cd7 to
5a82425
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/models/mentors_watch_for_question_creator.rb (3)
12-22: 同一トランザクション内での timestamp を揃えると微妙な差分が出にくいです
Time.currentをループ内で都度呼ぶとミリ秒レベルで差が出ます。1回だけ取得して使い回すと見た目も監査上の扱いも安定します。以下の小さなリファクタを検討ください。
def watch_records(question) - User.mentor.map do |mentor| + now = Time.current + User.mentor.map do |mentor| { watchable_type: 'Question', watchable_id: question.id, - created_at: Time.current, - updated_at: Time.current, + created_at: now, + updated_at: now, user_id: mentor.id } end end
10-13: ヘルパーメソッドは private にして意図を明確化
watch_recordsは外部公開不要なのでprivate化しておくと API 面の誤用が減ります。end - - def watch_records(question) + private + + def watch_records(question)
8-10: ユニークインデックスを活かした upsert_all へのリファクタリング提案watches テーブルにはすでに以下のユニークインデックスが存在するため、
insert_allのままでは重複レコードが発生する可能性があります。Rails 6.1+ かつユニークインデックスありの前提で、upsert_allに置き換え、空配列時のガードを追加すると安全です。・該当箇所
- ファイル:
app/models/mentors_watch_for_question_creator.rb- 行: 8–10
- watch_question_records = watch_records(question) - Watch.insert_all(watch_question_records) # rubocop:disable Rails/SkipsModelValidations + watch_question_records = watch_records(question) + return if watch_question_records.empty? + # ユニークキー: (watchable_type, watchable_id, user_id) + Watch.upsert_all( + watch_question_records, + unique_by: %i[watchable_type watchable_id user_id] + ) # rubocop:disable Rails/SkipsModelValidations– db/migrate/20230419022757_add_index_to_watches.rb にて以下が定義済みです
add_index :watches, [:watchable_type, :watchable_id, :user_id], unique: true
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/controllers/questions_controller.rb(2 hunks)app/models/ai_answer_creator.rb(1 hunks)app/models/mentors_watch_for_question_creator.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/controllers/questions_controller.rb
- app/models/ai_answer_creator.rb
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/mentors_watch_for_question_creator.rb
⏰ 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/mentors_watch_for_question_creator.rb (2)
4-4: ActiveSupport::Notifications 用の5引数シグネチャへの移行、問題なしAS::Notifications の購読コールバックと整合しており、未使用引数にプレフィックスを付けている点も良いです。
5-6: payload[:question] は常に Question インスタンスですので、追加ガードは不要です
QuestionsController のinstrument('question.create', { question: @question })／instrument('question.update', { question: @question })呼び出しでは常にquestion:キーに@question（Question オブジェクト）が含まれており、本サブスクライバもこれらのイベントのみを対象としているため、payload[:question]が nil になるケースや型が異なるケースは想定されません。現状のままで問題ありません。
|
@sjabcdefin 再度すみませんが、conflictの修正をお願いします。 |
…sに移行 - MentorsWatchForQuestionCreatorクラスをActiveSupport::Notificationsのインターフェースに対応 - QuestionsControllerのイベント発火にActiveSupport::Notifications.instrumentを追加 - active_support_notifications.rbにMentorsWatchForQuestionCreatorの登録を追加 - newspaper.rbからMentorsWatchForQuestionCreatorの登録を削除
- AIAnswerCreatorクラスをActiveSupport::Notificationsのインターフェースに対応 - active_support_notifications.rbにAIAnswerCreatorの登録を追加 - newspaper.rbからAIAnswerCreatorの登録を削除
複数行になる場合は通常のif文で書くの望ましいため
同じインスタンスを複数作成する必要がないため
sjabcdefin
force-pushed
the
chore/convert-mentorwatch-and-aianswer-to-notifications
branch
from
5a82425 to
7e178f0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/models/mentors_watch_for_question_creator.rb (4)
8-10: 重複耐性と並行性を強化するためinsert_all→upsert_all(unique_by:)を推奨通知は再送され得るため、同一レコードの多重挿入を DB 側で自然に弾けるようにしておくと堅牢です。
upsert_allにunique_by: %i[user_id watchable_type watchable_id]を指定すれば、ユニーク制約に基づく冪等性を確保できます（PostgreSQL 前提）。Rails/DB バージョン適合も合わせてご確認ください。- watch_question_records = watch_records(question) - Watch.insert_all(watch_question_records) # rubocop:disable Rails/SkipsModelValidations + watch_question_records = watch_records(question) + # 同一 (user_id, watchable_type, watchable_id) の多重挿入を防ぐ + Watch.upsert_all( + watch_question_records, + unique_by: %i[user_id watchable_type watchable_id] + ) # rubocop:disable Rails/SkipsModelValidations
12-22: Time.current を一度だけ評価して使い回し（微最適化＋タイムスタンプの一貫性）大量挿入時に
Time.currentを複数回呼ぶ必要はなく、1回だけ取得して全レコードに適用すると僅かながら効率的で、タイムスタンプのばらつきも避けられます。- def watch_records(question) - User.mentor.map do |mentor| + def watch_records(question) + now = Time.current + User.mentor.map do |mentor| { watchable_type: 'Question', watchable_id: question.id, - created_at: Time.current, - updated_at: Time.current, + created_at: now, + updated_at: now, user_id: mentor.id } end end
3-23: 配置の見直し（listener/subscriber としての役割を明確化）本クラスは ActiveRecord モデルではなくイベントリスナーの責務です。
app/models直下よりもapp/listeners（またはapp/subscribers）等へ配置すると役割が明確になり、Fat Model 回避・責務分離の観点でもガイドラインに沿います。既に同系のAIAnswerCreatorがあるなら、それと同一ポリシーの配置に揃えると可読性が上がります。
5-7: Watchable#watched? は全ユーザーの watch 存在を判定しており、メンターごとのチェックには不向きです現状の
question.watched?（app/models/concerns/watchable.rb:12–14）はdef watched? watches.present? endとして定義されており、任意のユーザーによる watch が 1 件でもあれば true を返します。そのため、以下のリスナーで早期 return が走ると、既に別ユーザーが watch しているだけで他のメンター向けの watch 作成処理が丸ごとスキップされてしまいます。
対象箇所
- app/models/concerns/watchable.rb 12–14 行目
- app/models/mentors_watch_for_question_creator.rb 5–7 行目
―――
提案する対応例（メンター単位の漏れ分のみ作成）
- Watchable モジュールに次のようなメソッドを追加
# app/models/concerns/watchable.rb def watched_by?(user) watches.exists?(user: user) end- リスナーを以下のように書き換え
# app/models/mentors_watch_for_question_creator.rb return if question.wip? question.mentors.each do |mentor| next if question.watched_by?(mentor) question.watches.create!(user: mentor) endこれにより、一部のメンターだけ watch が抜けているケースを正しく補完できます。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
app/controllers/questions_controller.rb(2 hunks)app/models/ai_answer_creator.rb(1 hunks)app/models/mentors_watch_for_question_creator.rb(1 hunks)config/initializers/active_support_notifications.rb(1 hunks)config/initializers/newspaper.rb(0 hunks)
💤 Files with no reviewable changes (1)
- config/initializers/newspaper.rb
🚧 Files skipped from review as they are similar to previous changes (3)
- config/initializers/active_support_notifications.rb
- app/controllers/questions_controller.rb
- app/models/ai_answer_creator.rb
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/mentors_watch_for_question_creator.rb
⏰ 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 (1)
app/models/mentors_watch_for_question_creator.rb (1)
4-4: ActiveSupport::Notifications の5引数コールバックに適合 — OK
def call(_name, _started, _finished, _unique_id, payload)への変更は AS::Notifications の購読インターフェイスに準拠しており問題ありません。
|
@komagata |
komagata
deleted the
chore/convert-mentorwatch-and-aianswer-to-notifications
branch
3 participants
Issue
概要
以下の2クラスの実装を
newspaperからActiveSupport::Notificationsに置き換えました。変更確認方法
1. ブランチの取り込みとサーバー起動
chore/convert-mentorwatch-and-aianswer-to-notificationsをローカルに取り込むforeman start -f Procfile.devでサーバーを立ち上げ2. 動作確認
質問作成時の動作確認
komagatamachidamentormentarounadmentorpjordMentorsWatchForQuestionCreatorの確認として、以下を確認するAIAnswerCreatorの確認として、以下を確認する。なお、任意のメンターユーザで一回確認すればOK。質問WIP時の動作確認
質問をWIPから公開した時の動作確認
komagatamachidamentormentarounadmentorpjordMentorsWatchForQuestionCreatorの確認として、以下を確認するAIAnswerCreatorの確認として、以下を確認する。なお、任意のメンターユーザで一回確認すればOK。Screenshot
※内部的なリファクタリングであり、見た目の変更はないためスクリーンショットはありません。
Summary by CodeRabbit
新機能
改善
リファクタ