Skip to content

Fix NoMethodError when a comment's commentable has no user#10281

Open
SAY-5 wants to merge 1 commit into
fjordllc:mainfrom
SAY-5:fix-mentioner-nil-commentable-user
Open

Fix NoMethodError when a comment's commentable has no user#10281
SAY-5 wants to merge 1 commit into
fjordllc:mainfrom
SAY-5:fix-mentioner-nil-commentable-user

Conversation

@SAY-5

@SAY-5 SAY-5 commented Jul 11, 2026

Copy link
Copy Markdown

Issue

概要

Mentioner#target_of_comment がハッシュリテラルで全ての項目を先に評価していたため、commentable_classReportProduct 以外でも commentable.user.login_name が実行されていました。commentable.usernilComment#usercommentable が user を持たない場合に nil を返します)のときに NoMethodError: undefined method 'login_name' for nil が発生します。

where_mention と同じく case に変更し、該当するクラスの分岐だけを評価するようにしました。あわせて Report / Product の分岐は user&.login_name にして user が無い場合でも落ちないようにしています。

変更確認方法

  1. {branch_name}をローカルに取り込む
  2. bin/rails test test/models/concerns/mentioner_test.rb を実行し、追加した #where_mention does not raise when the commentable has no user が通ることを確認する

Screenshot

変更前

(表示に影響のない内部処理の修正のため、スクリーンショットはありません)

変更後

Summary by CodeRabbit

  • バグ修正
    • ユーザー情報を持たない対象へのコメント処理で、エラーが発生しないよう改善しました。
    • 各対象種別に応じたコメント通知の表示内容を維持しつつ、処理の安定性を向上しました。

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

target_of_comment の分岐方式を変更し、Report のユーザー参照を nil 安全にしました。コメント対象にユーザーが存在しない場合でも例外が発生しないことをテストしています。

Changes

メンション対象の安全な生成

Layer / File(s) Summary
コメント対象の判定と nil ユーザー対応
app/models/concerns/mentioner.rb, test/models/concerns/mentioner_test.rb
target_of_comment のクラス判定を case 分岐に変更し、Report の user が nil の場合は安全に処理する。対応する where_mention のテストを追加する。

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed 変更内容の要点であるNoMethodError修正を簡潔に表しており、タイトルとして適切です。
Description check ✅ Passed Issue、概要、変更確認方法、Screenshotの主要セクションが揃っており、テンプレートに概ね沿っています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
test/models/concerns/mentioner_test.rb (1)

34-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

assert_nothing_raised だけでなく戻り値も検証してください。 comments(:comment1)Report なので safe navigation の経路自体は通っていますが、例外が出ないことだけでは where_mention の文言崩れを拾えません。usernil のときの文字列も assert_equal で固定してください。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/models/concerns/mentioner_test.rb` around lines 34 - 39, Update the
`where_mention` test to assert the returned string when `commentable.user` is
stubbed to nil, using `assert_equal` with the expected no-user message while
retaining the exception-safety check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/models/concerns/mentioner_test.rb`:
- Around line 34-39: Update the `where_mention` test to assert the returned
string when `commentable.user` is stubbed to nil, using `assert_equal` with the
expected no-user message while retaining the exception-safety check.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fbf54cb6-9953-4022-81f8-5740348715a1

📥 Commits

Reviewing files that changed from the base of the PR and between 18b8548 and 412bded.

📒 Files selected for processing (2)
  • app/models/concerns/mentioner.rb
  • test/models/concerns/mentioner_test.rb

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.

1 participant