研修生が提出物合格になった際、その研修生が所属する企業のアドバイザーにも通知の実装#9680
Merged
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughチェック承認時の件名／本文生成が受信者の役割（研修生 vs アドバイザー）に応じて変わり、チェック作成時に研修生の所属企業のアドバイザーへ追加通知を送るロジックが追加されました。メーラー、ノティファイア、ビュー、コールバック、テスト、フィクスチャが更新されています。 Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant User as チェック実行者
end
rect rgba(200,255,200,0.5)
participant CheckCallbacks as CheckCallbacks
participant Company as Company/Advisers
end
rect rgba(255,200,200,0.5)
participant ActivityDelivery as ActivityDelivery
participant ActivityNotifier as ActivityNotifier
participant Mailer as ActivityMailer/NotificationMailer
end
User->>CheckCallbacks: create Check
CheckCallbacks-->>Company: 条件判定（別ユーザーか／研修生か）
alt 研修生かつ企業のアドバイザー有り
Company->>CheckCallbacks: advisers一覧
CheckCallbacks->>ActivityDelivery: ActivityDelivery.with(check:, receiver:).notify(:checked) を複数送信
else 通常受信者のみ
CheckCallbacks->>ActivityDelivery: 既存の通知送信
end
ActivityDelivery->>ActivityNotifier: deliver checked
ActivityNotifier->>Mailer: 件名・本文を生成（kensyu_userで調整）
Mailer->>Company: メール/通知を配信
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
y-kawahara-gs
force-pushed
the
feature/add-advisor-notification-on-pass
branch
from
3635601 to
e1dc86e
Compare
y-kawahara-gs
force-pushed
the
feature/add-advisor-notification-on-pass
branch
5 times, most recently
from
da0a5d9 to
7747e29
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/mailers/activity_mailer.rb (1)
159-169:⚠️ Potential issue | 🟠 Major
checkedメールのリンク先が提出物ではありませんLine 164-166 の
@link_urlは受信者プロフィール (/users/:id) を向いたままです。app/views/activity_mailer/checked.html.slimの「この提出物へ」リンクもこれを使うので、今回追加したアドバイザー向けメールから提出物へ遷移できません。check.checkableのパスを使う形に揃えた方がよいです。💡 修正案
def checked(args = {}) `@check` ||= args[:check] `@receiver` ||= args[:receiver] `@user` = `@receiver` `@link_url` = notification_redirector_url( - link: "/users/#{`@user.id`}", + link: polymorphic_path(`@check.checkable`), kind: Notification.kinds[:checked] ) subject = "[FBC] #{`@check.checkable.user.login_name`}さんの#{`@check.checkable.title`}を#{`@check.action_label`}しました。"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/mailers/activity_mailer.rb` around lines 159 - 169, The checked method builds a profile URL for `@link_url` but the mail's "この提出物へ" should link to the submitted work; update the `@link_url` assignment in the checked method to point to the check's checkable resource (use the path for check.checkable rather than "/users/#{`@user.id`}") so the mail and the view (activity_mailer/checked.html.slim) both redirect to the submission; locate the checked method and replace the current notification_redirector_url(link: "/users/#{`@user.id`}", kind: Notification.kinds[:checked]) with a link based on `@check.checkable` (e.g., the checkable's show path or polymorphic path) and keep the same kind value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/mailers/activity_mailer_test.rb`:
- Around line 422-452: The test "trainee and advisers receive deffierent wording
in their acceptance emails" currently uses loose body assertions; after sending
the trainee email via ActivityMailer.checked(sender: check.sender, receiver:
check.receiver, check:), assert that the trainee email body does NOT include the
adviser-specific prefix (e.g. add assert_no_match(/kensyuさんの/, email.body.to_s)
or equivalent) and keep the existing positive matches for the trainee; then
after sending the adviser email via ActivityMailer.checked(... receiver:
check.receiver.company.advisers[0] ...), keep the existing
assert_match(/kensyuさんの「aptの基礎を覚える」/, email.body.to_s) to ensure the adviser
wording is present. This strengthens the test to guarantee the wording
difference between trainee and adviser emails.
---
Outside diff comments:
In `@app/mailers/activity_mailer.rb`:
- Around line 159-169: The checked method builds a profile URL for `@link_url` but
the mail's "この提出物へ" should link to the submitted work; update the `@link_url`
assignment in the checked method to point to the check's checkable resource (use
the path for check.checkable rather than "/users/#{`@user.id`}") so the mail and
the view (activity_mailer/checked.html.slim) both redirect to the submission;
locate the checked method and replace the current
notification_redirector_url(link: "/users/#{`@user.id`}", kind:
Notification.kinds[:checked]) with a link based on `@check.checkable` (e.g., the
checkable's show path or polymorphic path) and keep the same kind value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ec08712-76ae-48a2-9409-3547eb83cee3
📒 Files selected for processing (9)
app/mailers/activity_mailer.rbapp/mailers/notification_mailer.rbapp/models/check_callbacks.rbapp/notifiers/activity_notifier.rbapp/views/activity_mailer/checked.html.slimtest/fixtures/checks.ymltest/fixtures/products.ymltest/mailers/activity_mailer_test.rbtest/system/notification/products_test.rb
y-kawahara-gs
force-pushed
the
feature/add-advisor-notification-on-pass
branch
from
7747e29 to
8f7a3aa
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/mailers/activity_mailer_test.rb (1)
440-443: アドバイザー取得は固定 fixture を使うと安定します。
check.receiver.company.advisers[0]は関連の並び順に依存します。ここは期待値でも固定しているusers(:senpai)を直接渡した方が、同じ会社の adviser が増えたときも壊れにくいです。♻️ 修正案
ActivityMailer.checked( sender: check.sender, - receiver: check.receiver.company.advisers[0], + receiver: users(:senpai), check: ).deliver_now🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mailers/activity_mailer_test.rb` around lines 440 - 443, The test uses a positional association check.receiver.company.advisers[0] which is order-dependent; replace that with the fixed fixture user users(:senpai) to make the test stable—update the ActivityMailer.checked call arguments (sender:, receiver:, check:) to pass receiver: users(:senpai) so the test no longer relies on advisers ordering and continues to reference the expected adviser fixture.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/mailers/activity_mailer_test.rb`:
- Around line 440-443: The test uses a positional association
check.receiver.company.advisers[0] which is order-dependent; replace that with
the fixed fixture user users(:senpai) to make the test stable—update the
ActivityMailer.checked call arguments (sender:, receiver:, check:) to pass
receiver: users(:senpai) so the test no longer relies on advisers ordering and
continues to reference the expected adviser fixture.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a192b8b-86c2-4798-9a1a-a04184415cb9
📒 Files selected for processing (4)
test/fixtures/checks.ymltest/fixtures/products.ymltest/mailers/activity_mailer_test.rbtest/system/notification/products_test.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- test/fixtures/checks.yml
y-kawahara-gs
force-pushed
the
feature/add-advisor-notification-on-pass
branch
from
8f7a3aa to
15ee483
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/fixtures/checks.yml (1)
53-55: フィクスチャキーのタイポ:procuct75→product75キー名が
procuct75_check_komagataとなっていますが、product75のスペルミスです。ただし、既存のフィクスチャ（lines 29, 33, 37, 41, 45, 49）も同様に
procuctというスペルミスのパターンで統一されているため、現時点では一貫性が保たれています。将来的に一括修正することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixtures/checks.yml` around lines 53 - 55, The fixture key contains a typo: rename the key procuct75_check_komagata to product75_check_komagata in the YAML so it matches the referenced checkable product75 (Product); locate and update the key name in test/fixtures/checks.yml (and optionally plan a follow-up to batch-rename the other procuctNN keys for consistency) so tests reference the correctly spelled product75 identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/mailers/activity_mailer_test.rb`:
- Line 422: Rename the test method name to fix the typo: change the test
declaration string in the test 'trainee and advisers receive deffierent wording
in their acceptance emails' to use "different" instead of "deffierent" so it
reads 'trainee and advisers receive different wording in their acceptance
emails'; update the test name in activity_mailer_test.rb (the test block for
this case) accordingly.
---
Nitpick comments:
In `@test/fixtures/checks.yml`:
- Around line 53-55: The fixture key contains a typo: rename the key
procuct75_check_komagata to product75_check_komagata in the YAML so it matches
the referenced checkable product75 (Product); locate and update the key name in
test/fixtures/checks.yml (and optionally plan a follow-up to batch-rename the
other procuctNN keys for consistency) so tests reference the correctly spelled
product75 identifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0efe24a-9625-4231-9db0-5b9867984c35
📒 Files selected for processing (4)
test/fixtures/checks.ymltest/fixtures/products.ymltest/mailers/activity_mailer_test.rbtest/system/notification/products_test.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- test/fixtures/products.yml
- test/system/notification/products_test.rb
test/mailers/activity_mailer_test.rb
Outdated
| assert_match(/合格/, email.body.to_s) | ||
| end | ||
|
|
||
| test 'trainee and advisers receive deffierent wording in their acceptance emails' do |
There was a problem hiding this comment.
テスト名のタイポ: deffierent → different
テスト名に綴りミスがあります。
📝 修正案
- test 'trainee and advisers receive deffierent wording in their acceptance emails' do
+ test 'trainee and advisers receive different wording in their acceptance emails' do🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/mailers/activity_mailer_test.rb` at line 422, Rename the test method
name to fix the typo: change the test declaration string in the test 'trainee
and advisers receive deffierent wording in their acceptance emails' to use
"different" instead of "deffierent" so it reads 'trainee and advisers receive
different wording in their acceptance emails'; update the test name in
activity_mailer_test.rb (the test block for this case) accordingly.
15ee483 to
e259bd7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/mailers/activity_mailer_test.rb (1)
422-422:⚠️ Potential issue | 🟡 Minorテスト名のタイポ:
defferent→differentテスト名にまだスペルミスが残っています。「defferent」ではなく「different」が正しいです。
📝 修正案
- test 'trainee and advisers receive defferent wording in their acceptance emails' do + test 'trainee and advisers receive different wording in their acceptance emails' do🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mailers/activity_mailer_test.rb` at line 422, Rename the test method name to fix the typo: change "trainee and advisers receive defferent wording in their acceptance emails" to "trainee and advisers receive different wording in their acceptance emails" in the test definition (the test '...' do block) so the test name uses the correct spelling "different".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/mailers/activity_mailer_test.rb`:
- Line 422: Rename the test method name to fix the typo: change "trainee and
advisers receive defferent wording in their acceptance emails" to "trainee and
advisers receive different wording in their acceptance emails" in the test
definition (the test '...' do block) so the test name uses the correct spelling
"different".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 32adbfea-cf37-400e-8dcb-097bbc5ebf70
📒 Files selected for processing (4)
test/fixtures/checks.ymltest/fixtures/products.ymltest/mailers/activity_mailer_test.rbtest/system/notification/products_test.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- test/fixtures/products.yml
- test/fixtures/checks.yml
y-kawahara-gs
force-pushed
the
feature/add-advisor-notification-on-pass
branch
2 times, most recently
from
8d01729 to
43ba444
Compare
Contributor
Author
Contributor
|
@y-kawahara-gs |
matuaya
approved these changes
Mar 14, 2026
Contributor
matuaya
left a comment
matuaya
left a comment
There was a problem hiding this comment.
動作確認しましたのでApproveさせていただきたいと思います！
ちょっと細かいかもしれませんが、もしかしたらkensyu_userの変数名はユーザーオブジェクトが入っているように感じるかもしれないと思いました💦
でもこのままでも全然問題ないと思います🙇♀️
y-kawahara-gs
force-pushed
the
feature/add-advisor-notification-on-pass
branch
from
43ba444 to
b359a33
Compare
Contributor
Author
Contributor
Author
|
@komagata |
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
3 participants
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
概要
研修生の提出物が合格した際にアドバイザーにも合格通知の送る機能を実装
変更確認方法
｛｛メンター}}さんが提出物「OS X Mountain Lionをクリーンインストールする」を合格にしました。になっているかを確認する｛｛メンター}}さんが{{研修生の名前}}の提出物「OS X Mountain Lionをクリーンインストールする」を合格にしました。になっているかを確認するScreenshot
Summary by CodeRabbit