Q&Aページの未解決タブおよびグローバルメニューに表示する未解決質問数の表示にキャッシュを使う#9221
Q&Aページの未解決タブおよびグローバルメニューに表示する未解決質問数の表示にキャッシュを使う#9221
Conversation
Product.self_assigned_no_replied_product_idsメソッドをリファクタリング
FAQ作成ページのフォームの順番を変更する
- 管理ページヘッダーにあったボタンをお試し期間ページ内のヘッダー(`header.page-main-header`)に移動した - 移動先のクラスに合わせてボタンのCSSを`page-main-header`に変更した
- 他のお試し延長画面(edit/index)に合わせて、`header.page-main-header`内に配置されているボタンに適用するCSSは`page-main-header-actions`に統一する
…er-check 「日報を確認」と「提出物を合格にする」ボタンが確認後も表示されている
uncheckedページのcontrollerを整理した
Miya096jp
changed the title
Miya096jp
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/models/question.rb (1)
79-87: キャッシュ利用とプラクティス別ロジックは妥当です。
.sizeは未ロード時に SQL の COUNT を発行するため、性能上の問題はありません。コードベースで他箇所に.countを使っているので、可読性・一貫性の観点で任意に.countへ置き換えることをおすすめします。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/models/cache.rb(1 hunks)app/models/question.rb(1 hunks)app/views/application/_global_nav.slim(1 hunks)
🧰 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/question.rbapp/models/cache.rb
🧬 Code graph analysis (1)
app/models/question.rb (1)
app/models/cache.rb (1)
not_solved_question_count(55-59)
🔇 Additional comments (2)
app/views/application/_global_nav.slim (1)
38-40: キャッシュへの切り替えが適切に実装されています。未解決質問数の表示をDBクエリからキャッシュ値
Cache.not_solved_question_countに変更しており、同ファイル内の他の通知バッジ（未確認日報数、未割当提出物数）と一貫したパターンで実装されています。条件分岐ロジックも維持されており、問題ありません。app/models/cache.rb (1)
55-59: WIP質問の除外ロジックが正しく追加されています。
Question.not_solved.not_wip.countを使用することで、WIP状態の質問を未解決数から除外する実装になっており、他のキャッシュメソッド（unchecked_report_countなど）と一貫したパターンです。キャッシュキー名も適切で、対応する削除メソッドdelete_not_solved_question_countも存在しています。
markdown-it-purifierに問題があるために一時的に外している これが下に戻ったときにこのテストも戻す。
textareaのsanitizeのテストをスキップした
…utton 未チェックの日報を一括で開くボタンを復活させた
Miya096jp
force-pushed
the
chore/use-cache-for-unsolved-badge
branch
from
4ef8df9 to
742f571
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/controllers/api/answers_controller.rb(1 hunks)app/controllers/api/correct_answers_controller.rb(2 hunks)app/javascript/initializeAnswer.js(0 hunks)app/models/answer_cache_destroyer.rb(1 hunks)app/models/cache.rb(1 hunks)app/models/question.rb(1 hunks)app/models/question_auto_closer.rb(1 hunks)app/views/application/_global_nav.slim(1 hunks)
💤 Files with no reviewable changes (1)
- app/javascript/initializeAnswer.js
🚧 Files skipped from review as they are similar to previous changes (2)
- app/models/question.rb
- app/views/application/_global_nav.slim
🧰 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/question_auto_closer.rbapp/controllers/api/answers_controller.rbapp/controllers/api/correct_answers_controller.rbapp/models/answer_cache_destroyer.rbapp/models/cache.rb
🧠 Learnings (1)
📚 Learning: 2025-09-15T07:39:39.350Z
Learnt from: ryufuta
PR: fjordllc/bootcamp#9155
File: app/controllers/api/correct_answers_controller.rb:20-22
Timestamp: 2025-09-15T07:39:39.350Z
Learning: API::CorrectAnswersController#update では、ベストアンサー取り下げという単純な状態変更のため、update! を使用して DB エラー時に 500 を返し、成功時はデフォルトレスポンスを使用するのが適切。クライアント側のバリデーションエラーは発生しにくく、明示的なエラーハンドリングは不要。
Applied to files:
app/controllers/api/answers_controller.rbapp/controllers/api/correct_answers_controller.rb
🧬 Code graph analysis (1)
app/models/answer_cache_destroyer.rb (1)
app/models/cache.rb (1)
delete_not_solved_question_count(62-64)
⏰ 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/question_auto_closer.rb (1)
100-107: LGTM!イベントペイロードに
actionフィールドを追加する実装は正確です。__method__を使用してメソッド名を取得し、クラス名と組み合わせることで、イベントの発生元を明確に識別できます。app/controllers/api/answers_controller.rb (1)
47-55: ベストアンサー削除時のみイベントを発行する条件分岐Line 48の条件分岐により、
CorrectAnswerの削除時のみanswer_destroyイベントが発行されます。これは未解決質問数に影響するため、キャッシュクリアが必要なケースを適切に絞り込んでいます。通常の
Answerの削除では未解決質問数に影響しないため、この条件分岐は正しい実装です。app/controllers/api/correct_answers_controller.rb (1)
6-18: LGTM!
createアクションのイベントペイロードにactionフィールドが正しく追加されています。@answer変数も適切に使用されています。app/models/cache.rb (1)
56-59: キャッシュクリアの推奨
Question.not_solved.not_wip.countに変更されたことでWIP質問が除外されます（scope :not_wip, -> { where(wip: false) }）。既存のキャッシュ値と結果が異なる可能性があるため、デプロイ時にnot_solved_question_countキャッシュをクリアしてください。app/models/answer_cache_destroyer.rb (1)
6-8:actionキーの存在を確認済み
:answer_saveおよび:answer_destroyのすべてのNewspaper.publish呼び出しでaction:が指定されています。
| def update | ||
| answer = @question.answers.find(params[:answer_id]) | ||
| answer.update!(type: '') | ||
| Newspaper.publish(:answer_save, { answer: @answer }) | ||
| Newspaper.publish(:answer_save, { | ||
| answer: @answer, | ||
| action: "#{self.class.name}##{action_name}" | ||
| }) | ||
| end |
There was a problem hiding this comment.
変数の不一致によるバグ
Line 22でanswerをローカル変数として定義していますが、Line 25のイベント発行では@answerを使用しています。updateアクションでは@answerが設定されていないため、nilがペイロードに含まれることになります。
以下のdiffを適用して、正しい変数を使用するように修正してください:
def update
answer = @question.answers.find(params[:answer_id])
answer.update!(type: '')
Newspaper.publish(:answer_save, {
- answer: @answer,
+ answer: answer,
action: "#{self.class.name}##{action_name}"
})
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def update | |
| answer = @question.answers.find(params[:answer_id]) | |
| answer.update!(type: '') | |
| Newspaper.publish(:answer_save, { answer: @answer }) | |
| Newspaper.publish(:answer_save, { | |
| answer: @answer, | |
| action: "#{self.class.name}##{action_name}" | |
| }) | |
| end | |
| def update | |
| answer = @question.answers.find(params[:answer_id]) | |
| answer.update!(type: '') | |
| Newspaper.publish(:answer_save, { | |
| answer: answer, | |
| action: "#{self.class.name}##{action_name}" | |
| }) | |
| end |
🤖 Prompt for AI Agents
In app/controllers/api/correct_answers_controller.rb around lines 21 to 28, the
update action defines a local variable answer but publishes @answer (which is
nil); change the publish call to use the local answer (or assign @answer =
answer before publishing) so the payload contains the updated record; ensure
Newspaper.publish passes answer: answer and keep the action metadata unchanged.
10 participants
ブランチ名の修正のため、こちらのPRはクローズしました。
#9232 で引き続き対応を継続します。
Issue
Q&A画面の未解決タブのバッジにキャッシュを使う#8990
概要
#9202 <- キャッシュクリアをNewspaperからActiveSupport::Notificationに置換
備考
未回答の質問数は次のタイミングで変動するので、そのタイミングでキャッシュがクリアされ、新たにDBクエリー=>キャッシュされます。
補足
上記のタイミングの他に、本来キャッシュクリアすべきでないタイミング(質問への回答の投稿/更新/削除時)でも、不要なキャッシュクリアが実装されていますが、#9202で該当の部分をバグとして削除する予定のため、これらのタイミングでのキャッシュクリアの検証は不要です。
事前準備
管理者/メンター権限でログイン
ターミナルでログを監視する
確認方法
キャッシュを表示するパターン
次のタイミングでターミナルにログが出力されないことを確認してください
キャッシュクリア=>再キャッシュするパターン
以下のタイミングで、それぞれターミナルに次のログが出力されることを確認してください
[CACHE MISS] Executing DB query for not_solved_question_count質問の自動クローズ時のキャッシュクリア=>再キャッシュのパターン
Summary by CodeRabbit