Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughチェック文言を、チェック対象が Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/deliveries/activity_delivery_test.rb (1)
21-21:.notify(:graduated)のフィクスチャメッセージが不適切に変更されています。
ActivityNotifier#graduatedの本文は"🎉️ #{sender.login_name}さんが卒業しました！"ですが、このフィクスチャメッセージが「確認しました」→「合格にしました」に変更されており、卒業通知に「合格にしました」は意味的に無関係です。テストはデリバリー数のみを検証しているためテストは通過しますが、一括置換で意図せず変更されたと思われます。🛠️ 修正案
- message: "#{users(:kimura).login_name}さんがxxxxを合格にしました。", + message: "#{users(:kimura).login_name}さんが卒業しました！",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/deliveries/activity_delivery_test.rb` at line 21, フィクスチャの `.notify(:graduated)` 用メッセージが誤って「合格にしました」になっているので、ActivityNotifier#graduated の本文と一致する "🎉️ #{sender.login_name}さんが卒業しました！" に戻してください（該当のテストの fixture メッセージ行、および users(:kimura) を参照している箇所を更新）。これにより通知種別 `.notify(:graduated)` と ActivityNotifier#graduated の期待本文が整合します。
🤖 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/deliveries/activity_delivery_test.rb`:
- Line 21: フィクスチャの `.notify(:graduated)`
用メッセージが誤って「合格にしました」になっているので、ActivityNotifier#graduated の本文と一致する "🎉️
#{sender.login_name}さんが卒業しました！" に戻してください（該当のテストの fixture メッセージ行、および
users(:kimura) を参照している箇所を更新）。これにより通知種別 `.notify(:graduated)` と
ActivityNotifier#graduated の期待本文が整合します。
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
y-kawahara-gs
force-pushed
the
feature/trainee-submission-pass-notification-changed-from-checked-to-passed
branch
from
b47138b to
727acb7
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/notifiers/activity_notifier.rb (1)
253-256:action_name変数名がRailsの組み込みメソッドと衝突する可能性 / DRY違反変数名について
action_nameはAbstractController::Baseで定義されているRailsの組み込みメソッドです（ActionMailerのビューコンテキストでも利用可能）。ノーティファイアのコンテキストでは直接影響は小さいですが、action_labelやcheck_actionのようにより具体的な名前の方が意図が明確になります。DRY違反について
同じ三項条件式
checkable_type == "Product" ? "合格に" : "確認"が少なくとも以下の3箇所に重複しています：
app/notifiers/activity_notifier.rb（Line 253）app/views/activity_mailer/checked.html.slim（Line 1）app/mailers/activity_mailer.rb（AIサマリーによる）この判定ロジックを
Checkモデルにメソッドとして集約することで、将来的な変更を一箇所で管理できます。♻️ Checkモデルへの抽出例
Checkモデルに以下のメソッドを追加：# app/models/check.rb + def action_label + checkable_type == "Product" ? "合格に" : "確認" + endノーティファイアでの使用：
- action_name = check.checkable_type == "Product" ? "合格に" : "確認" - notification( - body: "#{check.sender.login_name}さんが#{check.checkable.title}を#{action_name}しました。", + body: "#{check.sender.login_name}さんが#{check.checkable.title}を#{check.action_label}しました。",ビューでの使用：
- - action_name = `@check.checkable_type` == "Product" ? "合格に" : "確認" + - action_label = `@check.action_label`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/notifiers/activity_notifier.rb` around lines 253 - 256, 変数名 `action_name` が Rails の組み込みメソッドと衝突する可能性と同じ三項条件式が複数箇所に重複しているので、三項判定 `check.checkable_type == "Product" ? "合格に" : "確認"` を Check モデルにメソッドとして抽出（例えば `action_label` または `check_action` を追加）し、`app/notifiers/activity_notifier.rb` の該当箇所では `action_name` を使わず `check.action_label` に置き換え、同様に `app/views/activity_mailer/checked.html.slim` と `app/mailers/activity_mailer.rb` で重複している判定をすべて `check.action_label` を呼ぶように変更してください。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/notifiers/activity_notifier.rb`:
- Around line 253-256: 変数名 `action_name` が Rails
の組み込みメソッドと衝突する可能性と同じ三項条件式が複数箇所に重複しているので、三項判定 `check.checkable_type == "Product"
? "合格に" : "確認"` を Check モデルにメソッドとして抽出（例えば `action_label` または `check_action`
を追加）し、`app/notifiers/activity_notifier.rb` の該当箇所では `action_name` を使わず
`check.action_label` に置き換え、同様に `app/views/activity_mailer/checked.html.slim` と
`app/mailers/activity_mailer.rb` で重複している判定をすべて `check.action_label`
を呼ぶように変更してください。
|
@matuaya |
|
@y-kawahara-gs |
matuaya
left a comment
matuaya
left a comment
There was a problem hiding this comment.
動作バッチリだと思いました！
Approveさせていただきます
|
@okuramasafumi |
| @@ -1,6 +1,6 @@ | |||
| = render '/notification_mailer/notification_mailer_template', | |||
| title: "#{@check.checkable.title}が確認されました", | |||
| title: "#{@check.checkable.title}が#{@check.action_label}されました", | |||
There was a problem hiding this comment.
「合格にされました」という日本語になることがありそうですが、確認済みでしたら大丈夫です！
There was a problem hiding this comment.
@okuramasafumi さん
レビューありがとうございます！
文言について、@machidaさんと確認していなかったので、確認して修正します！
machida
deleted the
feature/trainee-submission-pass-notification-changed-from-checked-to-passed
branch
4 participants
Issue
概要
研修生の提出物が合格になったときに、「{メンター}が{提出物}を確認しました」という通知の文言を「{メンター}が{提出物}を合格にしました」という文言に変更
変更確認方法
Screenshot
変更前
アプリ内通知
メール通知
変更後
アプリ内通知
メール通知
Summary by CodeRabbit