Skip to content

ペアワークをサポートする機能を実装#9626

Merged
komagata merged 209 commits intomainfrom
feature/support_pair_work_latest
Mar 2, 2026
Merged

ペアワークをサポートする機能を実装#9626
komagata merged 209 commits intomainfrom
feature/support_pair_work_latest

Conversation

@mousu-a
Copy link
Contributor

@mousu-a mousu-a commented Feb 7, 2026

こちらのPRが肥大化したため作り直しました🙏

Issue

概要

現在ディスコードで行っているペアプロ(ペアワーク)の募集、申し込みをbootcampアプリ上で行える機能を実装しました!

仕様をこちらのページにまとめています。

https://www.notion.so/2cd7324eb1cb807ea8fdde29addd6b1a

このPRでやること

  • ペアワークのCRUD機能の実装
  • ペアワーク作成時の通知関連の実装

このPRでやらないこと

  • ペアワーク機能の本番環境へのリリース(後述するペアワークの追加機能issueが終わり次第、本番環境へリリースします)
    • ペアワークの追加機能issueが終わるまでは、機能をfeature flagによって隠します。
  • スケジュールの変更機能
    • 現在ペアワークのtitledescriptionなどは変更できますが、スケジュールは変更できないようになっています。
  • ペア取り消し機能、それに付随する通知関連

このPRでやらないことに関してはこちらのissueで対応します。

変更確認方法

  1. feature/support_pair_workをローカルに取り込む
    1. git fetch origin feature/support_pair_work_latest
    2. git checkout feature/support_pair_work_latest
  2. Develop環境でのDiscord通知の確認方法の設定を行う(後ほど通知の確認で必要になります)
  3. bin/devでローカルサーバーを立ち上げる。
  4. kimuraでログイン
  5. http://localhost:3000/current_user/edit
    主な活動予定時間セクションにて、(月)3:00、(水)4:00、(木)3:00をチェックし、ページ下部の"更新する"ボタンをクリック
  6. サイドバーのQ&A ペアワークロゴをクリック。ペアワークのタブをクリックし一覧画面へ
  7. 右上の「ペアを募集する」をクリックし、新規作成画面へ
  8. 各項目を入力する
    1. プラクティスはなんでも良いです🙆‍♂️(選択しなくても問題はありません)
    2. 例:lsコマンド実装ペアプロ募集!
    3. 例:Rubyで ls コマンドのクローンを作成する課題に取り組んでいます!UNIXコマンドの基本動作をRubyで再現しながら、ファイル操作やオプション処理の理解を深めたい方、ぜひ一緒に取り組みましょう。
  9. スケジュール表に、ユーザーの登録情報である"主な学習時間"に該当する日にちにあらかじめチェックが入っていることを確認する。((月)3:00、(水)4:00、(木)3:00)
    1. 当日の、現在時刻より前の時間帯はチェックがつきません
  10. 画面下部の「登録する」ボタンを押しペアワークを新規作成する
  11. こちらでメール通知の確認
    1. メンター(敬称略:new-mentorpjord, komagata,unadmentor,machidanohimitsu,mentormentaro)に通知が来ていることを確認
  12. 先ほど作成したディスコードサーバーに通知が来ていることを確認
  13. 先ほど新規作成した内容のshow画面を確認
  14. 画面下部の「内容修正」ボタンを押してedit画面へ
  15. 各項目を編集し「更新する」ボタンを押してアップデートできることを確認
  16. komagataで再度ログイン(通知の確認もしたいためメンターが良いです)
    1. 右上の通知を確認し、ペアワーク作成時のサイト内通知が来ていることを確認
  17. 通知からshowへ飛ぶ
  18. スケジュール表の✅をクリックし、ペアワークを申し込み、ペア確定する
    1. 右上の通知からサイト内通知が来ていることを確認
    2. こちらにて、ペアワーク作成者(kimura)、メンター(敬称略:new-mentorkomagata,machidanohimitsu,mentormentaro,unadmentor,pjord)にペア確定の通知が来ていることを確認
    3. 先ほど作成したディスコードサーバーに通知が来ていることを確認
  19. 右上の「ペアワーク一覧」をクリック、ペア確定タブをクリック。先ほどのレコードがペア確定になっているのを確認
  20. rails db:seedを実行し、kimuraで再ログイン、ダッシュボードにアクセス
  21. 近日開催のペアワークが表示されているのを確認

Screenshot

変更前

新機能なので変更前のスクリーンショットはありません!

変更後

一覧
スクリーンショット 2025-06-01 20 55 18
詳細
スクリーンショット 2025-06-01 20 55 59
詳細ペア確定後
スクリーンショット 2025-06-01 20 58 30
近日開催のペアワーク
image

Summary by CodeRabbit

  • New Features

    • ペアワーク機能を追加(募集作成・スケジュール提案・予約・マッチング・一覧・詳細・編集・削除)
    • メンター/ウォッチャー向け自動通知(メール・サイト内通知・チャット)
    • ダッシュボードに今後のペアワーク表示・検索にペアワークを追加
  • Style

    • グローバルナビ、ページヘッダー、タブ、イベントのアクション領域、カードやフォーム周りの表示・レイアウト調整
    • フォームヘルプや一部マージン/変数の微調整

@coderabbitai
Copy link

coderabbitai bot commented Feb 7, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

ペアワーク機能を新規追加:モデル/スケジュール/コールバック/コントローラ/ルーティング/ビュー/ヘルパー/通知(notifier/mailer/activity)/スタイル/マイグレーション/テストが導入・更新された。

Changes

Cohort / File(s) Summary
モデル・DB / コア
app/models/pair_work.rb, app/models/pair_work_schedule.rb, app/models/pair_work_callbacks.rb, app/models/pair_work_notifier.rb, app/models/pair_work_matching_notifier.rb, app/models/watch_for_pair_work_creator.rb, db/migrate/...create_pair_works.rb, db/migrate/...create_schedules.rb, db/migrate/...add_channel_to_pair_work.rb, db/migrate/...add_not_null_constraint_to_pair_works_description.rb, db/migrate/...add_default_to_pair_works_wip.rb, db/migrate/...add_index_to_pair_works_published_at.rb, db/migrate/...rename_schedules_to_pair_work_schedules.rb, db/migrate/...add_unique_index_to_pair_work_schedules.rb, db/schema.rb
PairWork とスケジュールのモデル、コールバック、Notifier を追加。DB テーブル/カラム/制約/インデックスを導入。
コントローラ・ルート
app/controllers/pair_works_controller.rb, config/routes.rb, app/controllers/home_controller.rb, app/controllers/pair_works/reservations_controller.rb
RESTful ペアワークコントローラと予約コントローラ、ルーティング追加。HomeController#index に upcoming_pair_works を割当。
ビュー/パーシャル/コンポーネント
app/views/pair_works/*, app/views/home/*, app/views/questions_and_pair_works/*, app/components/sub_tab_component.html.slim, app/components/page_tabs_component.html.slim, app/views/application/_global_nav.slim, app/views/*/_tabs.html.slim, app/views/events/_participation.html.slim, app/views/regular_events/_participation.html.slim
一覧/詳細/作成/編集フォーム、スケジュールグリッド、ヘッダ等多数のパーシャル追加。タブ関連のルート要素を nav.page-tabs に変更、イベント参加UIを再構成。
ヘルパー・ユーティリティ
app/helpers/pair_work_helper.rb, app/helpers/sub_tabs/pair_works_helper.rb, app/helpers/page_tabs/questions_and_pair_works_helper.rb, app/helpers/application_helper.rb
スケジュール計算、checkbox id 生成、サブタブ/ページタブヘルパー、pair_work_available? を追加。
通知・メール・配信
app/mailers/activity_mailer.rb, app/notifiers/activity_notifier.rb, config/initializers/active_support_notifications.rb, app/notifiers/*, test/deliveries/activity_delivery_test.rb, app/views/activity_mailer/*
came_pair_work / matching_pair_work の ActivityMailer / ActivityNotifier を追加。ActiveSupport::Notifications 登録、配信ロジックとテンプレート・テストを追加。
キャッシュ・既存モデル拡張
app/models/cache.rb, app/models/user.rb, app/models/notification.rb, app/models/concerns/mentioner.rb, app/models/concerns/watchable.rb, app/models/unfinished_data_destroyer.rb, app/models/searcher/configuration.rb
未解決ペアワーク用キャッシュ追加、User に has_many :pair_works、Notification enum に新種別追加、メンション/watchable/検索設定へ PairWork を追加、未完了データ削除に PairWork を含める変更。
スタイル(CSS)
app/assets/stylesheets/application.css, app/assets/stylesheets/application/blocks/pair-work/_pair-work-info.css, app/assets/stylesheets/application/blocks/pair-work/_pair-work-schedule-dates.css, app/assets/stylesheets/application/blocks/page/_page-body-header.css, app/assets/stylesheets/application/blocks/event/_event-main-actions.css, app/assets/stylesheets/atoms/_a-card.css, app/assets/stylesheets/atoms/_a-form-help.css, app/assets/stylesheets/config/variables/_css-variables.css, app/assets/stylesheets/shared/blocks/_global-nav.css
ペアワーク関連の新規 CSS 追加と既存コンポーネント調整(global-nav サイズ/ラベル、event actions の再構成、カード条件削除、フォームヘルプ間隔調整)。
フィクスチャ/シード/テスト
db/fixtures/*pair_works*.yml, test/fixtures/*pair_works*.yml, db/seeds.rb, test/* (モデル・ヘルパー・mailer・配信・system 等)
フィクスチャ/シードに pair_works 系追加。PairWork に対する単体・ヘルパー・mailer・配信・システムテストを多数追加。

Sequence Diagram

sequenceDiagram
    participant User as ユーザー
    participant Browser as ブラウザ(UI)
    participant Controller as PairWorksController
    participant Model as PairWork
    participant Notifications as ActiveSupport::Notifications
    participant Delivery as ActivityDelivery

    User->>Browser: ペアワーク作成フォーム送信
    Browser->>Controller: POST /pair_works (params)
    Controller->>Model: PairWork.create(params)
    Model->>Notifications: instrument 'pair_work.create' (payload)
    Notifications->>Delivery: PairWorkNotifier / WatchForPairWorkCreator invoked
    Delivery->>User: メール・サイト通知配信

    User->>Browser: 予約(マッチング)実行
    Browser->>Controller: POST /pair_works/:id/reservations (reserved_at, buddy_id)
    Controller->>Model: pair_work.reserve(params)
    Model->>Notifications: instrument 'pair_work.reserve' (payload)
    Notifications->>Delivery: PairWorkMatchingNotifier invoked
    Delivery->>Watchers: マッチング通知配信
Loading

見積もられたコードレビュー工数

🎯 4 (Complex) | ⏱️ ~75 分

Possibly related issues

Possibly related PRs

Suggested reviewers

  • komagata

Poem

🐰 ペアワーク畑で跳ねるぼく、
日付を並べてぴたり合えば、
WIPが晴れて「マッチ!」と鳴り、
通知はぴょんと届くよ、メンターへ、
うさぎは小躍りして祝福する。

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.30% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PRタイトルは「ペアワークをサポートする機能を実装」で、変更セットの主な目的(ペアワークのCRUD機能と通知機能の実装)を明確に反映しており、簡潔でわかりやすい。
Description check ✅ Passed PR説明は必須テンプレートの全セクション(Issue、概要、変更確認方法、Screenshot)を含み、詳細な動作確認手順とスクリーンショットが添付されているため、要件を満たしている。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/support_pair_work_latest

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot requested a review from komagata February 7, 2026 03:13
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@app/models/pair_work.rb`:
- Line 53: Line 53の columns_for_keyword_search :title, :description 呼び出しは
ransackable_attributes の明示的定義(PairWork#ransackable_attributes
の定義行)によって上書きされるため削除してください; include Searchable は残して after_commit
コールバックや検索ヘルパー等の他機能に影響するのでそのままにし、不要な columns_for_keyword_search 呼び出しだけを取り除いてください。

In `@app/views/pair_works/_form.html.slim`:
- Around line 42-43: `- if pair_work.new_record?`
ブロック内のスケジュールセクションで子要素のインデントがずれているため、`- if pair_work.new_record?` の直下にある
`.form-item` を他の同レベル要素(例:既存の `.form-item` / `.row` と同じ 6→8
スペースの深さ)に合わせてインデントを修正してください。また、チャネル入力の `value: 'ペアワーク・モブワーク1'`
は編集時に既存値を上書きするので、`value:` オプションを削除するか、もしくは値を設定する処理を `if pair_work.new_record?`
条件内に移動して「新規作成時のみデフォルト値をセット」するように修正してください。
🧹 Nitpick comments (28)
app/views/events/_participation.html.slim (1)

23-43: event.capacity > event.participants.count が3回繰り返されています。

Line 23(class属性)、Line 26、Line 35 で同じ条件が評価されています。TODOコメントで認識済みのようですが、ローカル変数に結果をキャッシュするか、ヘルパーに移すと可読性とパフォーマンスの両方が改善されます。

♻️ 例: ローカル変数の活用
 - elsif event.opening?
+  - has_vacancy = event.capacity > event.participants.count
-  .event-main-actions.is-unparticipationed(class="#{event.capacity > event.participants.count ? 'is-available' : 'is-capacity-over'}")
+  .event-main-actions.is-unparticipationed(class="#{has_vacancy ? 'is-available' : 'is-capacity-over'}")
     .event-main-actions__body
       .event-main-actions__description
-        - if event.capacity > event.participants.count
+        - if has_vacancy

以降の条件分岐でも has_vacancy を使用できます。

db/migrate/20250414013258_add_channel_to_pair_work.rb (1)

1-5: デフォルト値のハードコードについて確認

チャンネル名 'ペアワーク・モブワーク1' がDBレベルのデフォルト値としてハードコードされています。チャンネル名が将来変更された場合、別途マイグレーションが必要になります。運用上問題がなければこのままで構いませんが、変更頻度が高い場合はアプリケーション層でのデフォルト設定(モデルの attribute メソッドや定数化)も検討できます。

app/views/questions_and_pair_works/_questions_and_pair_works_page_main_header.html.slim (1)

12-19: params[:action] よりも action_name の使用を推奨

Railsビューでは、現在のアクション名を取得するには action_name ヘルパーを使うのがイディオマティックです。params[:action] でも動作しますが、action_name の方がRailsの慣例に沿っており、意図が明確です。

♻️ 修正案
-              - unless params[:action] == 'new'
+              - unless action_name == 'new'
                 li.page-header-actions__item
                   = link_to new_question_path, class: 'a-button is-md is-secondary is-block' do
                     i.fa-regular.fa-plus
                     span 質問する
-              - unless params[:action] == 'index'
+              - unless action_name == 'index'
                 li.page-header-actions__item
                   = link_to questions_path(target: 'not_solved'), class: 'a-button is-md is-secondary is-block is-back' do
                     | Q&A一覧
app/views/pair_works/_form.html.slim (1)

3-5: .form-item の二重ネストが冗長に見える

Line 4 の .form-item の直下にまた .form-item(Line 5)がネストされています。外側の .form-item はスタイリング上意図的かもしれませんが、他の .form-item ブロック(Line 16, 26, 78)と同じレベルであるべきなら、Line 4 は不要な可能性があります。意図的なレイアウトであれば問題ありません。

app/views/application/_global_nav.slim (1)

52-70: 一時的な本番分岐のTODOコメントは適切ですが、重複コードに注意してください。

Lines 61-63 と Lines 64-70 の分岐で、Q&A ラベル部分が重複しています。現時点では一時的な実装のため許容範囲ですが、本番リリース時の削除漏れにご注意ください。

config/locales/ja.yml (1)

13-13: 日付フォーマットの追加は問題ありません。

mdw_uniquemdw_and_time のフォーマットが date.formatstime.formats にそれぞれ追加されています。%-m%-d でゼロパディングなしの表示になる点も意図通りかと思います。

一点確認:Line 25 の mdw_and_time: '%-m月%-d日(%a)%H:%M' は曜日と時刻の間にスペースがありませんが、6月5日(木)04:00 のような表示で意図通りでしょうか?%-m月%-d日(%a) %H:%M(スペースあり)の方が読みやすい可能性があります。

Also applies to: 25-25

app/models/concerns/mentioner.rb (1)

27-29: practice_title のロジックが Question と重複しています。

Lines 25-26 の Question 分岐と Lines 28-29 の PairWork 分岐で practice_title の算出ロジックが完全に同一です。現時点では2箇所のみなので許容範囲ですが、今後同様のパターンが増える場合はプライベートメソッドへの抽出を検討してください。

app/views/pair_works/_pair_work.html.slim (2)

43-45: pair_work.comments.length は全コメントをメモリにロードします

PR目標にN+1対策が含まれていますが、.length は常にコレクション全体をロードして配列の長さを返します。.size を使うと、プリロード済みならキャッシュされた値を、未ロードなら COUNT(*) クエリを利用します。

♻️ 提案する修正
-                | コメント(#{pair_work.comments.length})
+                | コメント(#{pair_work.comments.size})

28-29: do ブロック内のインデントが4スペースになっています

Line 29 のインデントが親要素から4スペースになっていますが、Slim の慣例では2スペースです。動作上は問題ない可能性がありますが、ファイル内の他の箇所と統一されていません。

♻️ 提案する修正
              = link_to pair_work.user, class: 'a-user-name' do
-                  = pair_work.user.long_name
+                = pair_work.user.long_name
db/fixtures/pair_work_schedules.yml (1)

9-11: 2.day2.days(複数形)に統一

プロジェクトの方針として、時間の加算には複数形(Numeric#hours, Numeric#days 等)を優先して用いることになっています。Line 3 の 1.day は単数で自然ですが、2.day2.days に修正した方が一貫性があります。

♻️ 提案する修正
 pair_work_schedule_3:
   pair_work: pair_work3
-  proposed_at: <%= Time.current.beginning_of_day + 2.day %>
+  proposed_at: <%= Time.current.beginning_of_day + 2.days %>

Based on learnings: 「fjordllc/bootcamp: 時間の加算には Numeric#hours(複数形)を優先して用いるプロジェクト方針」

app/models/pair_work_matching_notifier.rb (1)

7-7: 二重否定を unless に置き換えると可読性が向上します

return if !expr より return unless expr の方がRubyのイディオムに沿っています。

♻️ 提案
-    return if !pair_work.saved_change_to_attribute?(:reserved_at, from: nil, to: pair_work.reserved_at)
+    return unless pair_work.saved_change_to_attribute?(:reserved_at, from: nil, to: pair_work.reserved_at)
app/models/pair_work_notifier.rb (1)

15-19: User.mentor.each でN件の通知を逐次発行している点について

メンター数が増えた場合、eachループ内でActivityDeliveryを1件ずつ発行するとレスポンスタイムに影響する可能性があります。既存の通知パターンと同様であれば問題ありませんが、将来的にバックグラウンドジョブへの移行を検討する価値はあります。

app/models/pair_work_callbacks.rb (1)

4-11: if/elsifの両ブランチが同一処理のため、条件を統合できます。

両方のブランチで同じログ出力とキャッシュ削除を行っているため、条件をまとめるとよりシンプルになります。

♻️ 条件統合の提案
   def after_save(pair_work)
-    if pair_work.saved_change_to_attribute?(:published_at, from: nil)
-      Rails.logger.info '[CACHE CLEARED#after_save] Cache destroyed for unsolved pair work count.'
-      Cache.delete_not_solved_pair_work_count
-    elsif pair_work.saved_change_to_attribute?(:reserved_at) || pair_work.saved_change_to_attribute?(:wip)
+    if pair_work.saved_change_to_attribute?(:published_at, from: nil) ||
+       pair_work.saved_change_to_attribute?(:reserved_at) ||
+       pair_work.saved_change_to_attribute?(:wip)
       Rails.logger.info '[CACHE CLEARED#after_save] Cache destroyed for unsolved pair work count.'
       Cache.delete_not_solved_pair_work_count
     end
   end
test/helpers/pair_work_helper_test.rb (1)

51-52: テストブロック間に空行がありません。

他のテストブロック間には空行がありますが、learning_time_frame_checked?schedule_target_timeの間に空行がありません。

📝 修正提案
     assert_not learning_time_frame_checked?(past_date, my_learning_time_frame_id)
   end
+
   test 'schedule_target_time' do
app/views/pair_works/_header.html.slim (1)

16-16: pair_work.wippair_work.wip? の使い分けが不統一です。

Line 17では pair_work.wip? を使用していますが、Line 16のクラス条件では pair_work.wip が使われています。動作上は問題ありませんが、Railsの慣例に合わせて wip? に統一するとより明示的です。

♻️ 修正案
-      h1.page-content-header__title class=(pair_work.wip ? 'is-wip' : '')
+      h1.page-content-header__title class=(pair_work.wip? ? 'is-wip' : '')
app/models/watch_for_pair_work_creator.rb (1)

12-24: watch_records メソッドは private にすべきです。

watch_recordscall の内部実装の詳細であり、外部から呼ばれる必要がありません。同様のクラス(PairWorkNotifier, PairWorkMatchingNotifier)では private セクションにヘルパーメソッドが配置されています。

♻️ 修正案
   end
 
+  private
+
   def watch_records(pair_work)
db/migrate/20250203011220_create_pair_works.rb (2)

11-11: wip カラムにデフォルト値の追加を検討してください。

null: false が設定されていますが、デフォルト値がありません。モデル側で常に設定されるとしても、default: false を指定しておくとデータベースレベルでの安全性が向上します。


3-14: reserved_at へのインデックス追加を検討してください。

upcoming_pair_works スコープ等で reserved_at を条件にしたクエリが実行されるため、データ量が増えた場合にインデックスがあるとパフォーマンスが向上します。現段階ではデータ量が少ないと思われるので、必要に応じて後日対応でも問題ありません。

app/views/pair_works/_body.html.slim (2)

44-53: 認可制御をCSSクラス is-hidden のみで行っているのはセキュリティ上の防御層が薄い

is-hidden CSSクラスでフッター(編集・削除リンク)の表示を制御していますが、HTML自体はすべてのユーザーに配信されます。コントローラー側で set_my_pair_work による認可チェックがあるためサーバーサイドは保護されていますが、DOMにリンクやCSRFトークンが露出します。

Slim条件分岐で未認可ユーザーにはHTML自体をレンダリングしないほうがより安全です。

また、Line 51の current_user.admin? || pair_work.user == current_user は Line 44の条件と同一であり、フッター内にいる時点で常に真になるため冗長です。

♻️ 提案
- footer.card-footer class=(current_user == pair_work.user || current_user.admin? ? '' : 'is-hidden')
+ - if current_user == pair_work.user || current_user.admin?
+   footer.card-footer

88-92: TODOコメント: ペア確定取り消し機能が未実装

Line 90-92で「ペア確定を取り消す」ボタンが div 要素として描画されていますが、リンク先が未設定です。PR説明によると #8679 で対応予定とのことですが、現状このボタンをクリックしても何も起こりません。ユーザーの混乱を避けるため、disabled 属性の追加またはボタン自体の非表示を検討してください。

必要であれば、disabled状態のボタン実装を提案できます。

app/helpers/pair_work_helper.rb (2)

8-17: sorted_wdays はモジュラ演算で簡潔に書ける

現在のループは正しいですが、同等のロジックをワンライナーで表現できます。

♻️ 簡潔な実装案
  def sorted_wdays(date)
-   max_wday = 6
-   sorted_wdays = [date.wday]
-   max_wday.times do
-     next sorted_wdays << 0 if sorted_wdays.last == max_wday
-
-     sorted_wdays << sorted_wdays.last + 1
-   end
-   sorted_wdays
+   (0..6).map { |i| (date.wday + i) % 7 }
  end

19-25: disabled? メソッドの current_user 依存に注意

このヘルパーはビューコンテキストの current_user に暗黙的に依存しています。テスト(test/helpers/pair_work_helper_test.rb)では current_userusers(:kimura) としてスタブしていますが、ヘルパーメソッドの署名からこの依存が見えないため、将来的にDecoratorへの移行やユニットテスト時に問題になる可能性があります。

現時点では既存パターンに沿っているため問題ありませんが、留意点として記録します。

test/system/pair_works_test.rb (2)

26-28: Choices.jsのセレクタがハードコーディングされており脆弱

#choices--js-choices-practice-item-choice-12 は Choices.js が自動生成するIDであり、フィクスチャのデータ順やライブラリバージョンの変更で壊れる可能性があります。

テキストマッチ(text: 'sshdでパスワード認証を禁止にする')が既に併用されているため、IDが変わった場合でもテストが意図通りの要素を選択するか確認が必要です。


108-125: 認可テストはビュー層のみのカバレッジ

このテストは表示の有無を検証していますが、直接URLアクセスによる edit / destroy のサーバーサイド認可は検証されていません。コントローラーテストまたは追加のシステムテストで、未認可ユーザーが edit_pair_work_path に直接アクセスした場合の挙動(リダイレクトや404)もカバーすることを推奨します。

app/controllers/pair_works_controller.rb (1)

18-21: show アクションでN+1クエリの懸念

@pair_work = PairWork.find(params[:id]) では関連テーブルがeager loadされていません。ビュー(_body.html.slim)では pair_work.buddypair_work.userpair_work.practicepair_work.schedules にアクセスするため、個別クエリが発生します。

♻️ Eager loading の追加案
  def show
-   `@pair_work` = PairWork.find(params[:id])
+   `@pair_work` = PairWork.includes(:user, :buddy, :practice, :schedules).find(params[:id])
    `@comments` = `@pair_work.comments.order`(:created_at)
  end
db/schema.rb (1)

561-577: reserved_at カラムにインデックスの追加を検討

PairWork モデルでは solved / not_solved / not_held スコープが reserved_at をWHERE条件に使用しています。また by_target スコープやキャッシュカウント(Cache.not_solved_pair_work_count)でも頻繁にクエリされます。データ量が増加した際のパフォーマンスを考慮して、reserved_at へのインデックス追加を検討してください。

app/models/pair_work.rb (2)

45-51: upcoming_pair_works スコープのインデント・スタイルを修正

lambda本体のインデントが深すぎて可読性が低下しています。

♻️ インデント修正案
- scope :upcoming_pair_works, lambda { |user|
-                                now = Time.current
-                                within_day = now...(now + 3.days)
-                                PairWork.where(user_id: user.id).or(PairWork.where(buddy_id: user.id))
-                                        .solved
-                                        .where(reserved_at: within_day)
-                              }
+ scope :upcoming_pair_works, lambda { |user|
+   now = Time.current
+   within_day = now...(now + 3.days)
+   where(user_id: user.id).or(where(buddy_id: user.id))
+     .solved
+     .where(reserved_at: within_day)
+ }

なお、スコープ内では PairWork.where(...) の代わりに where(...) を使用できます(スコープのコンテキストは暗黙的にモデルクラスです)。


20-22: コールバックインスタンスの共有を検討

PairWorkCallbacks.new が3回呼ばれ、3つの別インスタンスが生成されています。PairWorkCallbacks はステートレスなので動作に問題はありませんが、1つのインスタンスを共有するとわずかにクリーンです。

♻️ 共有インスタンスの使用案
+ CALLBACKS = PairWorkCallbacks.new
+
- after_save PairWorkCallbacks.new
- before_destroy PairWorkCallbacks.new, prepend: true
- after_destroy PairWorkCallbacks.new
+ after_save CALLBACKS
+ before_destroy CALLBACKS, prepend: true
+ after_destroy CALLBACKS

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@app/views/application/_global_nav.slim`:
- Around line 48-71: The nav item's active-state regex only matches questions
paths (current_link(/^questions/)) so it stays inactive for pair_work pages;
update the call to include pair_works as well (e.g.
current_link(/^(questions|pair_works)/) or an equivalent regex) so the link
becomes active on both questions and pair_works routes; adjust the usage near
the pair_work_available? branching and references like
Cache.not_solved_pair_work_count to ensure consistent behavior.
🧹 Nitpick comments (4)
app/models/pair_work.rb (2)

20-22: コールバックごとに PairWorkCallbacks.new が生成される点について

after_savebefore_destroyafter_destroy それぞれで新しいインスタンスが生成されます。状態を持たないコールバッククラスであれば、定数として1つのインスタンスを共有する方が効率的です。

♻️ 改善案
+  PAIR_WORK_CALLBACKS = PairWorkCallbacks.new
+
-  after_save PairWorkCallbacks.new
-  before_destroy PairWorkCallbacks.new, prepend: true
-  after_destroy PairWorkCallbacks.new
+  after_save PAIR_WORK_CALLBACKS
+  before_destroy PAIR_WORK_CALLBACKS, prepend: true
+  after_destroy PAIR_WORK_CALLBACKS

47-53: スコープ内で PairWork.where を使用するとスコープチェーンが無視される

scope のラムダ内で PairWork.where(...) を使うと、呼び出し元で構築済みのリレーションチェーンが無視され、常に PairWork.all を起点とした新しいクエリが生成されます。where(...) を直接使う方がスコープの合成に対して安全です。

♻️ 改善案
   scope :upcoming_pair_works, lambda { |user|
-                                now = Time.current
-                                within_day = now...(now + 3.days)
-                                PairWork.where(user_id: user.id).or(PairWork.where(buddy_id: user.id))
-                                        .solved
-                                        .where(reserved_at: within_day)
-                              }
+    now = Time.current
+    within_day = now...(now + 3.days)
+    where(user_id: user.id).or(where(buddy_id: user.id))
+      .solved
+      .where(reserved_at: within_day)
+  }
app/views/pair_works/_form.html.slim (2)

4-5: .form-item の二重ネストは意図的ですか?

Line 4 と Line 5 で .form-item が二重にネストされています。外側の .form-item(Line 4)は子要素を包むだけで、内側の .form-item(Line 5)が実際のコンテンツを持ちます。不要なラッパーであれば削除を検討してください。


74-77: check_box_tag は未チェック時に値を送信しない点に注意

check_box_tagf.check_box と異なり、hidden field が自動生成されないため、未チェックの場合はパラメータに含まれません。スケジュール作成ではチェックされた日時のみ送信する意図なので問題ありませんが、編集画面でスケジュールの変更が必要になった場合(将来的に new_record? 条件を外す場合)、既存スケジュールの削除ができなくなるリスクがあります。現時点では if pair_work.new_record? で制御されているため問題ありません。

mousu-a and others added 25 commits February 11, 2026 09:18
_formのform_withの中にscheduleが入っていることで、レコードのupdate時に相手確定してしまう不具合が発生していた。
なのでedit画面の_formではscheduleを表示しないようにした(どちらにせよ仕様でscheduleは更新出来ない)
@mousu-a
Copy link
Contributor Author

mousu-a commented Feb 19, 2026

@komagata
再レビューをお願いします!
各コメント返信しています。

ご指摘はいただいていませんが一点気になるところを変更しました🙏

295923a

@@ -0,0 +1,7 @@
# frozen_string_literal: true

module PairWorkDecorator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,23 @@
- important = upcoming_pair_work.reserved_at.beginning_of_day == Time.current.beginning_of_day
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

booleanをpartial独自の変数として定義するより何らかのメソッドやヘルパーにするほうが良いように思いました。(極力viewにロジックが無い方がいいので)

spanのクラス名の部分と合わせてhelperとかにするといいかもとおもいました。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meta_label_by_statusとしてhelperに切り出しました!

@@ -0,0 +1,11 @@
pair_work_schedule_1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pair_work_schedule_1:
pair_work_schedule1:

他と合わせるとこっちのほうがいいかもです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正しました!

= render 'reactions/reactions', reactionable: pair_work

hr.a-border-tint
footer.card-footer class=(current_user == pair_work.user || current_user.admin? ? '' : 'is-hidden')
Copy link
Contributor Author

@mousu-a mousu-a Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@komagata

極力viewにロジックが無い方がいい

Suggested change
footer.card-footer class=(current_user == pair_work.user || current_user.admin? ? '' : 'is-hidden')
footer.card-footer class=(pair_work.owner_or_admin?(current_user) ? '' : 'is-hidden')

ここもこんな感じでメソッド化してしまおうと考えているんですが、これはやりすぎでしょうか?

current_user == object.user || current_user.admin?というようなコードは既存のコードでもよく見ますし、メソッド化してもコードの長さはそんなに変わっていません。

ですがこうすれば他のモデルでも同じ形で実装出来そうですし、メソッド化することで確実にわかりやすくなるかなと思いました。これだとどっちが良いでしょうか?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mousu-a このレベルでメソッド化するとこのサイト全体で膨大な数のメソッドが作成されることになるのでここまではやらなくてもいいかな〜と思いました。

Copy link
Contributor Author

@mousu-a mousu-a Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます!
ここはそのままにしておきます🙏

@mousu-a mousu-a force-pushed the feature/support_pair_work_latest branch 4 times, most recently from 365c94f to 0003da2 Compare February 22, 2026 02:25
時間を固定することに意味がなかったので境界値をテストするように
近日開催のペアワークは:今日開催される場合を確認できるように
@mousu-a mousu-a force-pushed the feature/support_pair_work_latest branch from 0003da2 to 0315198 Compare February 22, 2026 03:14
@mousu-a
Copy link
Contributor Author

mousu-a commented Feb 22, 2026

@komagata
再レビューをお願いします!
各コメント返信しています。

ご指摘はいただいていませんが気になるところを変更しました🙏
0003da2
5e75c6a

@komagata
Copy link
Member

@mousu-a

フィクスチャの修正
時間を固定することに意味がなかったので境界値をテストするように
近日開催のペアワークは:今日開催される場合を確認できるように

時間の固定は良いことだと思います。
Fixtureの時間を固定して、テスト時にtravel_toで現在時間も固定して境界値をテストした方が良いと思います。
Time.currentをそのままつかうのは避けたいです。

境界値をテストするように
@mousu-a
Copy link
Contributor Author

mousu-a commented Feb 23, 2026

@komagata
再レビューをお願いします!
境界値を設定するように変更し、travel_toで時間を固定しました。
近日開催のペアワークに関しては必要なためTime.currentを使う形としています。


Fixtureの時間を固定して、テスト時にtravel_toで現在時間も固定して境界値をテストした方が良いと思います。
Time.currentをそのままつかうのは避けたいです。

この変更はdb/fixtures/pair_works.ymlのものだったので関係ないものと思っていました。

以下の認識のすり合わせをさせていただきたいです🙇‍♂️
時間を固定しなくて問題ないものでも、とりあえず固定しておけば考えるコストを減らせるし、テストの再現性も高くなる。
そのためフィクスチャ、テストでは基本的に時間は固定しておくのが丸い、という認識で良かったでしょうか?

@komagata
Copy link
Member

komagata commented Feb 25, 2026

@mouse-a

時間を固定しなくて問題ないものでも、とりあえず固定しておけば考えるコストを減らせるし、テストの再現性も高くなる。
そのためフィクスチャ、テストでは基本的に時間は固定しておくのが丸い、という認識で良かったでしょうか?

はい。
ただ、固定しないと非常に再現しずらい不具合が出やすいので基本的には必ず固定したい感じです。

assert learning_time_frame_checked?(future_date, my_learning_time_frame_id)
assert_not learning_time_frame_checked?(past_date, my_learning_time_frame_id)
end
test 'schedule_target_time' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1行開けたほうがいいかもです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

すいません、漏れでした🙏
修正しました!

@mousu-a
Copy link
Contributor Author

mousu-a commented Mar 1, 2026

@komagata
再レビューをお願いします!

テストコードの空行に関してissueを立てたのですがこちらどうでしょうか?
#9707

@komagata
Copy link
Member

komagata commented Mar 2, 2026

@mousu-a

テストコードの空行に関してissueを立てたのですがこちらどうでしょうか?

Issueの作成ありがとうございます。とってもいいとおもいます〜

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認させていただきました。OKです〜🙆‍♂️

@komagata komagata merged commit 4049f55 into main Mar 2, 2026
7 checks passed
@komagata komagata deleted the feature/support_pair_work_latest branch March 2, 2026 07:20
@github-actions github-actions bot mentioned this pull request Mar 2, 2026
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants