Closed
Conversation
📝 WalkthroughWalkthrough変更前の主催者IDを捕捉してから主催者割当や参加取消を行い、差分を計算して新しい主催者へ Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as Controller
participant User as Userモデル
participant Event as RegularEventモデル
participant ASN as ActiveSupport::Notifications
participant Notifier as OrganizerCreateNotifier
participant ActivityNotifier as ActivityNotifier
participant Delivery as DeliveryQueue
participant Mailer as ActivityMailer
Controller->>User: capture_before_regular_event_organizers()
User-->>Controller: [{regular_event, before_organizer_ids}, ...]
Controller->>Event: 参加取消・主催者再割当（更新）
Event-->>Controller: 更新完了
Controller->>ASN: instrument "organizer.create" (payload)
ASN-->>Notifier: deliver payload
Notifier->>Event: load current organizer_ids
Notifier-->>Notifier: 差分計算→new_organizer_ids
Notifier->>ActivityNotifier: build create_organizer notifications (receiverごと)
ActivityNotifier->>Delivery: enqueue delivery jobs
Delivery->>Mailer: deliver email to receiver
Mailer-->>Delivery: 配信完了
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
yokomaru
force-pushed
the
feature/notify_new_appointed_organizer_when_regular_event_updates
branch
from
477a227 to
7bcc152
Compare
yokomaru
force-pushed
the
feature/notify_new_appointed_organizer_when_regular_event_updates
branch
from
7bcc152 to
99c0f94
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/fixtures/regular_event_repeat_rules.yml (1)
128-136: YAMLの重複キーを修正してください。
regular_event_repeat_rule43が2回定義されています（Lines 128-131とLines 133-136）。YAMLの仕様では、重複キーは後の定義で上書きされるため、regular_event41用のルールが失われます。新しいルールには
regular_event_repeat_rule44を使用してください。🐛 修正案
regular_event_repeat_rule43: frequency: 6 day_of_the_week: 4 regular_event: regular_event41 -regular_event_repeat_rule43: +regular_event_repeat_rule44: frequency: 6 day_of_the_week: 4 regular_event: regular_event42
🤖 Fix all issues with AI agents
In `@test/fixtures/organizers.yml`:
- Around line 49-55: The YAML fixture defines the mapping key organizer13 twice
which causes the second entry (user: kensyu, regular_event: regular_event42) to
overwrite the first (user: hatsuno, regular_event: regular_event39); fix by
giving each organizer a unique key (e.g., rename the second organizer13 to
organizer14 or another unused identifier) so both fixtures
(hatsuno/regular_event39 and kensyu/regular_event42) are preserved.
In `@test/fixtures/regular_events.yml`:
- Around line 253-262: The fixture regular_event42 has a mismatch between its
title ("研修生が主催しているイベント") and the user field (currently set to komagata); update
the user field to kensyu to match the title and organizers.yml association, or
alternatively add a clear comment next to regular_event42 stating that the event
belongs to komagata while kensyu is an organizer, so the intention is explicit;
locate regular_event42 in test/fixtures/regular_events.yml and make the
change/add comment accordingly.
In `@test/integration/training_completion_test.rb`:
- Around line 17-18: Rename the test method string in test 'training complite
triggers organizer_created notification and enqueues delivery' to correct the
typo to 'training complete triggers organizer_created notification and enqueues
delivery' (update the test declaration in
test/integration/training_completion_test.rb where the test name is defined) and
remove or clarify the vague comment "# modelで分ける" (either delete the comment if
unnecessary or replace it with a clear explanation of intent) so the test name
and inline comment are readable and accurate.
In `@test/mailers/activity_mailer_test.rb`:
- Around line 1288-1306: The test contains an unused call
ActivityMailer.with(regular_event:, receiver:, sender:).create_organizer that
neither assigns, delivers, nor asserts on the result; remove that line (or
alternatively replace it with a separate test that assigns the result to a
variable, calls deliver_now, and asserts on the delivered mail following the
pattern used in the other mailer tests) so only the exercised
ActivityMailer.create_organizer(...).deliver_now path remains in this test.
🧹 Nitpick comments (4)
app/controllers/training_completion_controller.rb (1)
57-65:notify_to_new_regular_event_organizerメソッドが複数箇所で重複しています。このメソッドは以下の3箇所で同一の実装が存在します：
app/controllers/hibernation_controller.rbapp/controllers/training_completion_controller.rb（本ファイル）app/models/retirement.rbDRY原則に従い、共通のモジュールやヘルパーに抽出することを検討してください。
♻️ リファクタリング案: Concernへの抽出
# app/controllers/concerns/regular_event_organizer_notifiable.rb module RegularEventOrganizerNotifiable extend ActiveSupport::Concern private def notify_to_new_regular_event_organizer(regular_events_and_organizers, sender:) regular_events_and_organizers.each do |regular_event_and_organizer| ActiveSupport::Notifications.instrument('organizer.create', regular_event: regular_event_and_organizer[:regular_event], before_organizer_ids: regular_event_and_organizer[:before_organizer_ids], sender:) end end end各コントローラーやモデルで
include RegularEventOrganizerNotifiableして利用できます。app/controllers/hibernation_controller.rb (1)
59-66: LGTM!notify_to_new_regular_event_organizerメソッドの実装は適切です。他のコントローラー/モデル（
Retirement#notify_to_new_regular_event_organizer、TrainingCompletionController#notify_to_new_regular_event_organizer）と同じパターンで実装されており、一貫性があります。任意のリファクタリング提案: この
notify_to_new_regular_event_organizerメソッドは、Retirement、TrainingCompletionController、そして本コントローラーで同一のロジックが繰り返されています。将来的に共通モジュールや concern への抽出を検討しても良いかもしれません。test/integration/hibernation_test.rb (1)
17-18: コメント「# modelで分ける」の意図を確認してください。このコメントはTODOまたは実装メモのように見えます。対応済みであれば削除するか、まだ対応が必要な場合は
# TODO:プレフィックスを付けて明確にすることを検討してください。app/models/retirement.rb (1)
108-115:notify_to_new_regular_event_organizerメソッドの重複について。このメソッドの同一実装が
HibernationControllerおよびTrainingCompletionControllerにも存在します。（sender が@userとcurrent_userで異なるのは、モデル内のインスタンス変数とコントローラのヘルパーメソッドの差異による）。将来的には、専用のNotifierクラスや共通モジュールへの抽出を検討する価値があります。ただし、コーディングガイドラインの「Serviceクラスの乱用を避ける」方針を踏まえ、現時点では各クラスの責務に沿った配置として許容範囲です。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
app/controllers/hibernation_controller.rbapp/controllers/regular_events_controller.rbapp/controllers/training_completion_controller.rbapp/mailers/activity_mailer.rbapp/models/notification.rbapp/models/organizer.rbapp/models/organizer_create_notifier.rbapp/models/retirement.rbapp/models/user.rbapp/notifiers/activity_notifier.rbapp/views/activity_mailer/create_organizer.html.slimconfig/initializers/active_support_notifications.rbtest/deliveries/activity_delivery_test.rbtest/fixtures/organizers.ymltest/fixtures/regular_event_repeat_rules.ymltest/fixtures/regular_events.ymltest/integration/hibernation_test.rbtest/integration/regular_event_test.rbtest/integration/retirement_test.rbtest/integration/training_completion_test.rbtest/mailers/activity_mailer_test.rbtest/mailers/previews/activity_mailer_preview.rbtest/models/organizer_create_notifier_test.rbtest/notifiers/activity_notifier_test.rb
🧰 Additional context used
📓 Path-based instructions (6)
app/views/**/*.slim
📄 CodeRabbit inference engine (AGENTS.md)
Place Slim templates in
app/views/and ensure they are linted according toconfig/slim_lint.yml
Files:
app/views/activity_mailer/create_organizer.html.slim
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indent for Ruby code, snake_case for method names, and CamelCase for class names as enforced by RuboCop
Files:
test/mailers/previews/activity_mailer_preview.rbtest/integration/training_completion_test.rbtest/integration/regular_event_test.rbapp/controllers/regular_events_controller.rbapp/models/organizer.rbapp/notifiers/activity_notifier.rbtest/mailers/activity_mailer_test.rbapp/mailers/activity_mailer.rbapp/models/retirement.rbtest/models/organizer_create_notifier_test.rbtest/notifiers/activity_notifier_test.rbtest/integration/retirement_test.rbapp/models/user.rbtest/deliveries/activity_delivery_test.rbapp/models/organizer_create_notifier.rbapp/models/notification.rbtest/integration/hibernation_test.rbconfig/initializers/active_support_notifications.rbapp/controllers/training_completion_controller.rbapp/controllers/hibernation_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/mailers/previews/activity_mailer_preview.rbtest/integration/training_completion_test.rbtest/integration/regular_event_test.rbapp/controllers/regular_events_controller.rbapp/models/organizer.rbapp/notifiers/activity_notifier.rbtest/mailers/activity_mailer_test.rbapp/mailers/activity_mailer.rbapp/models/retirement.rbtest/models/organizer_create_notifier_test.rbtest/notifiers/activity_notifier_test.rbtest/integration/retirement_test.rbapp/models/user.rbtest/deliveries/activity_delivery_test.rbapp/models/organizer_create_notifier.rbapp/models/notification.rbtest/integration/hibernation_test.rbconfig/initializers/active_support_notifications.rbapp/controllers/training_completion_controller.rbapp/controllers/hibernation_controller.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/mailers/previews/activity_mailer_preview.rbtest/integration/training_completion_test.rbtest/integration/regular_event_test.rbtest/mailers/activity_mailer_test.rbtest/models/organizer_create_notifier_test.rbtest/notifiers/activity_notifier_test.rbtest/integration/retirement_test.rbtest/fixtures/organizers.ymltest/deliveries/activity_delivery_test.rbtest/fixtures/regular_events.ymltest/integration/hibernation_test.rbtest/fixtures/regular_event_repeat_rules.yml
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests under matching
test/*directories with*_test.rbnaming convention
Files:
test/integration/training_completion_test.rbtest/integration/regular_event_test.rbtest/mailers/activity_mailer_test.rbtest/models/organizer_create_notifier_test.rbtest/notifiers/activity_notifier_test.rbtest/integration/retirement_test.rbtest/deliveries/activity_delivery_test.rbtest/integration/hibernation_test.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
app/**/*.rb: Place Rails app code inapp/directory with subdirectories formodels/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/
Follow Rails file naming conventions (e.g.,app/models/user.rb)
Files:
app/controllers/regular_events_controller.rbapp/models/organizer.rbapp/notifiers/activity_notifier.rbapp/mailers/activity_mailer.rbapp/models/retirement.rbapp/models/user.rbapp/models/organizer_create_notifier.rbapp/models/notification.rbapp/controllers/training_completion_controller.rbapp/controllers/hibernation_controller.rb
test/fixtures/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Store test fixtures in
test/fixtures/directory
Files:
test/fixtures/organizers.ymltest/fixtures/regular_events.ymltest/fixtures/regular_event_repeat_rules.yml
🧠 Learnings (10)
📓 Common learnings
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'）の場合でも必ず実行する必要がある。
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になることは想定されておらず、エラーハンドリングは不要である。
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: 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: 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: 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: 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:19-23
Timestamp: 2025-09-12T21:17:26.029Z
Learning: Rails アップグレードPRにおいて、config_for(:secrets) を使った webhook URL 設定の shared セクション未参照問題は、設定システム全体の変更として別PRで対応すべきである。
📚 Learning: 2026-01-11T04:41:41.597Z
Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 8566
File: app/views/pair_works/_body.html.slim:95-100
Timestamp: 2026-01-11T04:41:41.597Z
Learning: Ruby 3.1以降では、ハッシュ値省略記法により、render 'schedule', pair_work: という形式が有効です。これは render 'schedule', pair_work: pair_work と等価で、同名の変数がスコープ内に存在する場合にのみ使用できます。fjordllc/bootcamp プロジェクトではこのモダンな Ruby 構文が使用されているため、構文エラーとして誤って指摘しないことを推奨します。対象ファイルは Slim テンプレート（例: app/views/.../*.slim）全体に適用可能です。
Applied to files:
app/views/activity_mailer/create_organizer.html.slim
📚 Learning: 2025-07-15T12:58:02.735Z
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`が通知一覧のフィルタ対象ではないためで、意図的な設計判断である。
Applied to files:
test/integration/training_completion_test.rbapp/models/notification.rbtest/integration/hibernation_test.rbapp/controllers/training_completion_controller.rb
📚 Learning: 2025-09-01T22:31:57.345Z
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'）の場合でも必ず実行する必要がある。
Applied to files:
test/integration/training_completion_test.rbapp/models/retirement.rbtest/integration/retirement_test.rbtest/integration/hibernation_test.rbconfig/initializers/active_support_notifications.rbapp/controllers/training_completion_controller.rbapp/controllers/hibernation_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:
test/integration/training_completion_test.rbtest/models/organizer_create_notifier_test.rbapp/models/organizer_create_notifier.rbtest/integration/hibernation_test.rb
📚 Learning: 2025-09-01T22:31:57.345Z
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）必ず実行する必要がある。
Applied to files:
test/integration/training_completion_test.rbapp/models/retirement.rbtest/integration/retirement_test.rbtest/integration/hibernation_test.rbapp/controllers/hibernation_controller.rb
📚 Learning: 2025-07-15T12:33:04.885Z
Learnt from: reckyy
Repo: fjordllc/bootcamp PR: 8356
File: db/fixtures/users.yml:823-826
Timestamp: 2025-07-15T12:33:04.885Z
Learning: fjordllc/bootcampプロジェクトでは、`trainee: true`は「研修に所属していたユーザー」全体を示すフラグであり、研修中のユーザーと研修を終了したユーザーの両方を含む。そのため、`trainee: true`と`training_completed_at`の併存は意図された仕様であり、排他的な関係ではない。
Applied to files:
test/integration/training_completion_test.rb
📚 Learning: 2025-10-22T06:04:36.036Z
Learnt from: sjabcdefin
Repo: fjordllc/bootcamp PR: 9247
File: app/controllers/checks_controller.rb:0-0
Timestamp: 2025-10-22T06:04:36.036Z
Learning: ChecksController#createおよび#destroyでは、Checkの作成・削除とActiveSupport::Notifications.instrumentによるイベント発行(プラクティスのステータス更新)を同一トランザクション内で実行し、いずれかが失敗した場合は両方をロールバックする。これによりWebUI表示とDB状態の整合性を保証している。
Applied to files:
test/integration/training_completion_test.rb
📚 Learning: 2026-01-09T10:57:42.506Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T10:57:42.506Z
Learning: Applies to test/**/*_test.rb : Place unit and integration tests under matching `test/*` directories with `*_test.rb` naming convention
Applied to files:
test/integration/regular_event_test.rb
📚 Learning: 2026-01-08T21:12:18.350Z
Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 8566
File: app/controllers/pair_works_controller.rb:10-18
Timestamp: 2026-01-08T21:12:18.350Z
Learning: In fjordllc/bootcamp, when using ActiveRecord.order to specify ascending order, omit the explicit :asc and rely on Rails' default ASC behavior. Prefer .order(:published_at) over .order(published_at: :asc). If you need a non-default order (e.g., DESC), specify it explicitly. This guideline applies to Ruby/Rails controller and model files, especially in app/controllers and similar ActiveRecord usage sites.
Applied to files:
app/controllers/regular_events_controller.rbapp/controllers/training_completion_controller.rbapp/controllers/hibernation_controller.rb
🧬 Code graph analysis (12)
test/mailers/previews/activity_mailer_preview.rb (2)
app/notifiers/activity_notifier.rb (1)
create_organizer(419-433)app/mailers/activity_mailer.rb (1)
create_organizer(473-488)
test/integration/training_completion_test.rb (5)
test/models/organizer_create_notifier_test.rb (1)
include(5-57)test/integration/hibernation_test.rb (1)
include(5-44)test/integration/regular_event_test.rb (1)
include(5-42)test/integration/retirement_test.rb (1)
include(4-41)test/supports/notification_helper.rb (1)
notifications(4-6)
test/integration/regular_event_test.rb (4)
test/integration/training_completion_test.rb (1)
include(5-44)test/integration/hibernation_test.rb (1)
include(5-44)test/integration/retirement_test.rb (1)
include(4-41)test/supports/notification_helper.rb (1)
notifications(4-6)
app/notifiers/activity_notifier.rb (2)
app/mailers/activity_mailer.rb (1)
create_organizer(473-488)test/mailers/previews/activity_mailer_preview.rb (1)
create_organizer(190-195)
test/mailers/activity_mailer_test.rb (3)
app/notifiers/activity_notifier.rb (1)
create_organizer(419-433)app/mailers/activity_mailer.rb (1)
create_organizer(473-488)test/mailers/previews/activity_mailer_preview.rb (1)
create_organizer(190-195)
app/mailers/activity_mailer.rb (3)
app/notifiers/activity_notifier.rb (1)
create_organizer(419-433)test/mailers/previews/activity_mailer_preview.rb (1)
create_organizer(190-195)app/models/user.rb (1)
retired?(694-696)
app/models/retirement.rb (3)
app/controllers/hibernation_controller.rb (1)
notify_to_new_regular_event_organizer(59-66)app/controllers/training_completion_controller.rb (1)
notify_to_new_regular_event_organizer(58-65)app/models/user.rb (1)
capture_before_regular_event_organizers(961-968)
test/models/organizer_create_notifier_test.rb (2)
test/integration/training_completion_test.rb (1)
include(5-44)app/models/organizer_create_notifier.rb (2)
call(3-20)call(4-19)
test/notifiers/activity_notifier_test.rb (2)
test/deliveries/activity_delivery_test.rb (1)
test(5-526)app/notifiers/activity_notifier.rb (1)
create_organizer(419-433)
test/integration/hibernation_test.rb (4)
test/integration/training_completion_test.rb (1)
include(5-44)test/integration/regular_event_test.rb (1)
include(5-42)test/integration/retirement_test.rb (1)
include(4-41)test/supports/notification_helper.rb (1)
notifications(4-6)
app/controllers/training_completion_controller.rb (3)
app/models/user.rb (3)
capture_before_regular_event_organizers(961-968)cancel_participation_from_regular_events(886-888)delete_and_assign_new_organizer(890-892)app/models/retirement.rb (1)
notify_to_new_regular_event_organizer(108-115)app/controllers/hibernation_controller.rb (1)
notify_to_new_regular_event_organizer(59-66)
app/controllers/hibernation_controller.rb (3)
app/models/user.rb (3)
capture_before_regular_event_organizers(961-968)cancel_participation_from_regular_events(886-888)delete_and_assign_new_organizer(890-892)app/models/retirement.rb (1)
notify_to_new_regular_event_organizer(108-115)app/controllers/training_completion_controller.rb (1)
notify_to_new_regular_event_organizer(58-65)
🪛 YAMLlint (1.37.1)
test/fixtures/organizers.yml
[error] 53-53: duplication of key "organizer13" in mapping
(key-duplicates)
⏰ 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 (23)
app/models/organizer.rb (1)
11-11: 問題ありません。空行の追加による可読性向上のための変更です。動作への影響はありません。
config/initializers/active_support_notifications.rb (1)
38-38: LGTM!
organizer.createイベントのサブスクリプション追加は、既存のパターンに従っており適切です。app/views/activity_mailer/create_organizer.html.slim (1)
1-4: LGTM!共通テンプレートを適切に使用しており、必要なローカル変数も正しく渡されています。
app/models/user.rb (1)
961-968: LGTM!主催者の状態を変更前にキャプチャするメソッドとして適切に実装されています。コーディングガイドラインに従い、処理をUserモデルに移動することでFat Controllerを回避しています。
app/controllers/training_completion_controller.rb (1)
11-32: 研修完了時の主催者通知フローが適切に実装されています。
- 変更前の状態をキャプチャ
- 参加キャンセルと新規主催者の割り当て
- 通知の発行
の順序が正しく実装されています。
app/models/organizer_create_notifier.rb (1)
3-19: LGTM! 新規主催者への通知ロジックは正しく実装されています。ActiveSupport::Notifications のサブスクライバパターンに従った適切な実装です。
before_organizer_idsとafter_organizer_idsの差分を計算して新しい主催者を特定し、通知を送信するロジックは明確です。app/controllers/hibernation_controller.rb (1)
21-24: LGTM! 休会フローにおける主催者通知の統合は適切です。操作の順序（キャプチャ → 変更 → 通知）が正しく、
RetirementモデルやTrainingCompletionControllerと一貫したパターンで実装されています。app/mailers/activity_mailer.rb (1)
471-488: LGTM!create_organizerメーラーアクションは適切に実装されています。既存のメーラーアクション（
update_regular_eventなど）と一貫したパターンで実装されており、mail_notification?とretired?のチェックも正しく行われています。test/notifiers/activity_notifier_test.rb (1)
166-177: LGTM!#create_organizerのテストは適切に実装されています。既存のテスト（
#update_regular_eventなど）と一貫したパターンで、notify_nowとnotify_laterの両方をテストしています。test/mailers/previews/activity_mailer_preview.rb (1)
190-195: LGTM!プレビューメソッドが既存のパターンに従って正しく実装されています。
update_regular_eventなど他のメソッドと同様のスタイルで、ActivityMailer#create_organizerに必要なパラメータ（regular_event、receiver、sender）を適切に渡しています。test/integration/retirement_test.rb (1)
16-40: LGTM!退会フローで新しい主催者への通知が正しく発行されることを確認するインテグレーションテストが適切に実装されています。
perform_enqueued_jobsブロック内で退会処理を実行し、以下を検証しています：
- 退会ユーザーが定期イベントの主催者から削除される
- 新しい主催者（receiver）が追加される
create_organizer通知が作成されるテストケース名も英語で記述されており、コーディングガイドラインに準拠しています。
app/models/notification.rb (1)
46-47: LGTM!新しいenum値
training_completed: 27とcreate_organizer: 28が正しく連番で追加されています。learningsに基づくと、
training_completedがTARGETS_TO_KINDSに含まれていないのは意図的な設計判断です（通知一覧のフィルタ対象ではないため）。create_organizerも同様に、特定のユーザーへの直接通知であり、フィルタリング対象ではないため、TARGETS_TO_KINDSに含めない設計は妥当です。test/deliveries/activity_delivery_test.rb (1)
503-525: LGTM!
create_organizer通知のデリバリーテストが既存のパターンに従って正しく実装されています。4つのシナリオ（notify!、notify、with().notify!、with().notify）を網羅し、他のテストケース（例：update_regular_event、added_work）と一貫したスタイルで記述されています。app/notifiers/activity_notifier.rb (1)
418-433: LGTM!既存の通知メソッド（例:
update_regular_event）と一貫したパターンで実装されています。regular_event、receiver、senderを適切に抽出し、kind: :create_organizerで通知を作成しています。app/controllers/regular_events_controller.rb (1)
43-56: LGTM!
before_organizer_idsを更新前に取得し、更新後にorganizer.createイベントを発行する設計は適切です。これによりOrganizerCreateNotifierが新規追加された主催者を正確に特定できます。test/models/organizer_create_notifier_test.rb (1)
5-57: LGTM!テストケースは主要なシナリオを適切にカバーしています：
- 新規主催者追加時の通知
- 新規主催者なしの場合の通知なし
- 複数主催者追加時の通知
- 主催者交代時の通知
assert_differenceを使用してAbstractNotifier::Testing::Driver.enqueued_deliveries.countの変化を検証するアプローチは適切です。test/integration/training_completion_test.rb (1)
5-43: テスト構造は適切です。他の統合テスト（
hibernation_test.rb、retirement_test.rb、regular_event_test.rb）と一貫したパターンで実装されています。AbstractNotifier.delivery_modeの設定・復元、ログイン処理、通知の検証が適切に行われています。test/integration/hibernation_test.rb (2)
1-16: テストの構造は適切です。
setup/teardownでAbstractNotifier.delivery_modeを保存・復元するパターンは、RetirementTestやTrainingCompletionTestと一貫しており、良い実装です。
28-43: テストケースの実装は適切です。
perform_enqueued_jobsでバックグラウンドジョブを実行し、以下を検証しています：
- 元のユーザーが
regular_eventから削除されること- 新しい主催者（receiver）が追加されること
create_organizer通知が作成されること他の統合テスト（
RetirementTest、TrainingCompletionTest）と一貫したパターンです。test/integration/regular_event_test.rb (2)
1-16: LGTM!テストの基本構造は他の統合テストと一貫しており、
AbstractNotifier.delivery_modeの保存・復元も適切です。
17-41: テストケースの実装は良好です。定期イベントの更新時に新しい主催者が追加され、
create_organizer通知が送信されることを正しく検証しています。user_idsの構築方法（シンボルから文字列IDへの変換）も明確です。app/models/retirement.rb (2)
32-32: トランザクション外での配置は適切です。
handle_regular_events_on_retirementがトランザクションブロックの外で呼ばれているのは正しい設計です。DB変更が成功した後に通知を送信するため、トランザクションのロールバック時に不要な通知が送られることを防いでいます。
117-122:handle_regular_events_on_retirementメソッドの実装は良好です。処理の順序が適切です：
- 変更前の主催者IDをキャプチャ
- イベント参加をキャンセル
- 主催者を削除し新しい主催者を割り当て
- 新しい主催者に通知
capture_before_regular_event_organizersを先に呼び出すことで、変更後に正しく差分を検出できます。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- 研修生が企画したイベントがフィクスチャになかったため追加
…し元で明示的に発行するように修正した
- FatControllerを避けるために以下も実装
- 現時点でのオーガーナイザー取得処理をUserモデルに移動
- イベント発行処理をprivateに移動
- メソッドが長すぎるため、定期イベントに関する処理はprivateメソッドにまとめた - 一時的に# rubocop:disable Metrics/ClassLengthを追加
yokomaru
force-pushed
the
feature/notify_new_appointed_organizer_when_regular_event_updates
branch
from
99c0f94 to
ef58954
Compare
komagata
deleted the
feature/notify_new_appointed_organizer_when_regular_event_updates
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
概要
変更確認方法
Screenshot
変更前
変更後
Summary by CodeRabbit
新機能
テスト
✏️ Tip: You can customize this high-level summary in your review settings.