コメント数の実装を見直し#10107
Merged
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughEventsHelper の Changesコメント数表示の統一化
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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.
Inline comments:
In `@app/helpers/events_helper.rb`:
- Around line 16-17: Replace the use of event.comments.length inside the
event_comment_count helper with event.comments.size to avoid loading the entire
association; update the method event_comment_count in
app/helpers/events_helper.rb to call event.comments.size (so it will use counter
cache or a COUNT query when appropriate) while leaving the existing safe_join
and content_tag logic intact.
In `@app/views/movies/_movie_header.html.slim`:
- Around line 58-61: 現在のテンプレートは .page-content-header-metas__meta 要素の内側で - if
movie.comments.any? を使っているため、コメントが0件でも空のラッパー要素だけが残ります。修正は条件分岐をラッパーごと外側へ移動し、- if
movie.comments.any? を .page-content-header-metas__meta の前に置いて、内部に a.a-meta と =
event_comment_count(movie)
を含めるようにしてください（参照シンボル：.page-content-header-metas__meta、movie.comments.any?、event_comment_count(movie)）。
In `@app/views/pages/_doc_header.html.slim`:
- Around line 61-64: 現在のテンプレート renders an empty wrapper when there are zero
comments because the condition page.comments.any? is inside the
.page-content-header-metas__meta block; move the conditional that checks
page.comments.any? to wrap the entire .page-content-header-metas__meta element
so the wrapper is not emitted when there are no comments, and keep the inner
link that calls event_comment_count(page) only when the condition is true.
In `@app/views/products/_product_header.html.slim`:
- Around line 73-76: The wrapper element .page-content-header-metas__meta should
only be rendered when comments exist: change the condition so the - if
product.comments.any? sits outside and wraps the entire
.page-content-header-metas__meta element (not just the a.a-meta), so the whole
meta container is omitted when there are zero comments; keep the inner link and
use the existing event_comment_count(product) helper as-is.
In `@app/views/regular_events/_regular_event.html.slim`:
- Around line 26-29: 現在のテンプレート renders .page-content-header-metas__meta even
when regular_event.comments.any? が false, leaving an empty wrapper; move the
conditional up so the wrapper is only rendered when there are comments: wrap the
entire .page-content-header-metas__meta and its child a.a-meta /
event_comment_count(regular_event) inside the if regular_event.comments.any?
check (i.e. place the if before the .page-content-header-metas__meta element) so
the div is omitted entirely when there are no comments.
In `@app/views/regular_events/_regular_events.html.slim`:
- Around line 50-54: Slimテンプレートでコメント表示の条件が不適切な位置にあり空の .card-list-item-meta__item
が出力されているので、現在の - if event.comments.any? を .card-list-item-meta__item
要素の外側ではなく内側（つまり .card-list-item-meta__item
を条件ブロック内に移動）にして、コメントがない場合は要素自体を出力しないように修正してください（参照シンボル: - if event.comments.any?
と event_comment_count）。
In `@app/views/talks/_talk.html.slim`:
- Around line 12-18: The hr element hr.card-list-item__row-separator is a void
element but currently has indented child lines (.card-list-item__row,
.card-list-item-meta__items, etc.), which produces invalid HTML; fix
_talk.html.slim by unindenting the lines that follow
hr.card-list-item__row-separator so that .card-list-item__row and its
descendants are siblings of the hr (same indentation level) rather than
children, preserving the existing markup structure under .card-list-item__row
and not changing class names.
In `@test/system/announcements_test.rb`:
- Line 47: テストのコメント数アサーションがページ内の任意の span にマッチしてしまうため、検証対象を show.html.slim
でコメント数を出している a.a-meta(href='`#comments`') 配下の span.is-emphasized に限定してください（該当テスト:
test/system/announcements_test.rb, アサーション箇所: assert_selector 'span', text: '2'
を修正）。具体的には現在の汎用的な selector を a.a-meta[href="`#comments`"] の下の span.is-emphasized
を狙うセレクタに置き換え、期待値 '2' を引き続き検証するようにしてください（参照シンボル: event_comment_count, a.a-meta,
span.is-emphasized）。
In `@test/system/movies_test.rb`:
- Line 50: The current assertion assert_selector 'span', text: 1 is too broad;
narrow the selector to target the comment count rendered by
events_helper#event_comment_count by asserting on the link and emphasized span
it produces (the a.a-meta[href='`#comments`'] wrapper and its span.is-emphasized
child). Update the assertion in test/system/movies_test.rb to use a selector
like "a.a-meta[href='`#comments`'] span.is-emphasized" with text: 1 so the test
specifically checks the comment-count element.
In `@test/system/pages_test.rb`:
- Line 49: The test currently asserts a loose span text which may match other
"1" instances; scope the assertion to the comment-count container by checking
the span.is-emphasized inside the comment link/container (the output produced by
event_comment_count). For example, target the selector path such as
a.a-meta[href='`#comments`'] .is-emphasized or .card-list-item-meta__item .a-meta
.is-emphasized (or use a within block around a.a-meta[href='`#comments`'] and
assert_selector 'span.is-emphasized', text: '1') so the assertion only validates
the comment count element.
In `@test/system/talks/user_list_test.rb`:
- Line 54: The assertion `assert_text '1'` inside the
within('.card-list-item-meta') scope is matching the date "2019年01月02日…" instead
of the comment count; update the test to target the exact element rendered by
event_comment_count by asserting against the span with class is-emphasized (e.g.
use within('.card-list-item-meta') then assert_selector 'span.is-emphasized',
text: '1' or assert_text only inside that selector) so the test checks the
comment count element specifically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcfbbbe9-f708-43f0-aff1-14ff62914193
📒 Files selected for processing (30)
app/assets/stylesheets/atoms/_a-meta.cssapp/components/products/product_component.html.slimapp/helpers/events_helper.rbapp/views/announcements/_announcement.html.slimapp/views/announcements/show.html.slimapp/views/companies/products/_product.html.slimapp/views/events/_event.html.slimapp/views/events/_events.html.slimapp/views/home/_colleague_trainee.html.slimapp/views/movies/_movie_header.html.slimapp/views/pages/_doc_header.html.slimapp/views/pages/_page.html.slimapp/views/pair_works/_pair_work.html.slimapp/views/products/_product.html.slimapp/views/products/_product_header.html.slimapp/views/products/_product_list_item.html.slimapp/views/regular_events/_regular_event.html.slimapp/views/regular_events/_regular_events.html.slimapp/views/reports/_product.html.slimapp/views/reports/_report.html.slimapp/views/reports/_report_header.html.slimapp/views/talks/_talk.html.slimapp/views/users/events/index.html.slimapp/views/users/regular_events/index.html.slimtest/system/announcements_test.rbtest/system/movies_test.rbtest/system/pages_test.rbtest/system/product/product_list_test.rbtest/system/reports_test.rbtest/system/talks/user_list_test.rb
Contributor
Author
|
@koguchi-e |
komagata
force-pushed
the
main
branch
2 times, most recently
from
b420541 to
f31c31e
Compare
Contributor
|
@zecky1120 １週間以内に対応させていただく予定ですが… こちらは他PRでも発生しているため、Discordにて確認中です。 似たような現象が起きているPR
こちらの原因が判明次第改めて対応させていただきます。 |
Contributor
|
@zecky1120 ryufutaさんからご解答いただきまして、下記手順を実行したところ、コミットが消えました。 git switch main
git pull --rebase origin main
git switch 該当ブランチ
git rebase main
git push --force-with-lease origin 該当ブランチ |
zecky1120
force-pushed
the
bug/review-of-event_comment_count
branch
from
2e0c079 to
cba3c71
Compare
Contributor
Author
|
@koguchi-e |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
koguchi-e
approved these changes
Jun 7, 2026
koguchi-e
left a comment
koguchi-e
left a comment
Contributor
There was a problem hiding this comment.
確認しました！
バッチリです🙆問題なさそうでした！
Contributor
Author
|
@koguchi-e @okuramasafumi |
okuramasafumi
left a comment
okuramasafumi
left a comment
Contributor
There was a problem hiding this comment.
ヘルパーはevent_comment_countという名前になっていますが、見たところeventとの強い関連性は特になさそうなので、comment_count(commentable)くらいが適切に思えました。
数える実装をそろえたのはとても良いと思います！
| @@ -22,4 +22,4 @@ | |||
| - if user.comments.present? | |||
Contributor
Author
There was a problem hiding this comment.
@okuramasafumi
ご確認ありがとうございました！ メソッド名を変更しました。
メソッド名の変更に伴うviewsとany?も変更し、coderabbitにて指摘されたN+1クエリ対策も変更しました〜✌️
def9e24
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/helpers/events_helper.rb (1)
16-18: ⚡ Quick winより適切な配置先へ移動を検討してください。
comment_countは汎用的なメソッドですがEventsHelperに配置されています。このメソッドはPage、Product、Event、RegularEventなど複数のモデルで利用されているため、ApplicationHelperへの移動を検討してください。コーディングガイドラインでは Fat Controller/Model を避け、適切な責務分担を推奨しています。汎用ヘルパーはアプリケーション全体で利用可能な場所に配置することで、モジュール間の結合度を下げ、保守性を向上できます。♻️ 移動例
app/helpers/application_helper.rbへ移動:# app/helpers/application_helper.rb module ApplicationHelper + def comment_count(commentable) + safe_join(['コメント（', content_tag(:span, commentable.comments.size, class: 'is-emphasized'), '）']) + end end# app/helpers/events_helper.rb module EventsHelper - def comment_count(commentable) - safe_join(['コメント（', content_tag(:span, commentable.comments.size, class: 'is-emphasized'), '）']) - end - def event_participant_count(event)Based on coding guidelines: まずFat Controllerを避け、次にFat Modelを避ける。適切な責務分担のため、複数モデルで利用される汎用ヘルパーは ApplicationHelper へ配置することを推奨。
🤖 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 `@app/helpers/events_helper.rb` around lines 16 - 18, The `comment_count` method is currently in EventsHelper but is a generic utility method used by multiple models (Page, Product, Event, RegularEvent), violating the single responsibility principle. Move the `comment_count` method from app/helpers/events_helper.rb to app/helpers/application_helper.rb so that it is accessible application-wide and reduces coupling between modules, following the coding guideline that generic helpers used across multiple models should be placed in ApplicationHelper.Source: Coding guidelines
🤖 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.
Inline comments:
In `@app/helpers/events_helper.rb`:
- Around line 16-18: The comment_count helper method in events_helper.rb calls
commentable.comments.size which triggers N+1 queries when comments are not
pre-loaded. Add eager loading to prevent this: in the talks, movies, and reports
controllers, add includes(:comments) or preload(:comments) to the index action
(or equivalent list display method) to pre-load the comments association,
matching the pattern already implemented in events, products, pages,
announcements, and pair_works controllers.
---
Nitpick comments:
In `@app/helpers/events_helper.rb`:
- Around line 16-18: The `comment_count` method is currently in EventsHelper but
is a generic utility method used by multiple models (Page, Product, Event,
RegularEvent), violating the single responsibility principle. Move the
`comment_count` method from app/helpers/events_helper.rb to
app/helpers/application_helper.rb so that it is accessible application-wide and
reduces coupling between modules, following the coding guideline that generic
helpers used across multiple models should be placed in ApplicationHelper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e27b3b1-d561-4021-a6aa-13c738fdf7c8
📒 Files selected for processing (23)
app/components/products/product_component.html.slimapp/helpers/events_helper.rbapp/views/announcements/_announcement.html.slimapp/views/announcements/show.html.slimapp/views/companies/products/_product.html.slimapp/views/events/_event.html.slimapp/views/events/_events.html.slimapp/views/home/_colleague_trainee.html.slimapp/views/movies/_movie_header.html.slimapp/views/pages/_doc_header.html.slimapp/views/pages/_page.html.slimapp/views/pair_works/_pair_work.html.slimapp/views/products/_product.html.slimapp/views/products/_product_header.html.slimapp/views/products/_product_list_item.html.slimapp/views/regular_events/_regular_event.html.slimapp/views/regular_events/_regular_events.html.slimapp/views/reports/_product.html.slimapp/views/reports/_report.html.slimapp/views/reports/_report_header.html.slimapp/views/talks/_talk.html.slimapp/views/users/events/index.html.slimapp/views/users/regular_events/index.html.slim
🚧 Files skipped from review as they are similar to previous changes (18)
- app/views/reports/_product.html.slim
- app/views/reports/_report.html.slim
- app/views/home/_colleague_trainee.html.slim
- app/views/reports/_report_header.html.slim
- app/views/pages/_doc_header.html.slim
- app/views/announcements/_announcement.html.slim
- app/views/companies/products/_product.html.slim
- app/views/pair_works/_pair_work.html.slim
- app/views/talks/_talk.html.slim
- app/views/announcements/show.html.slim
- app/views/regular_events/_regular_event.html.slim
- app/views/movies/_movie_header.html.slim
- app/views/users/regular_events/index.html.slim
- app/views/events/_event.html.slim
- app/views/events/_events.html.slim
- app/views/products/_product_list_item.html.slim
- app/views/products/_product_header.html.slim
- app/components/products/product_component.html.slim
Contributor
Author
|
@okuramasafumi @komagata |
Member
|
@zecky1120 マージしました〜 |
11 tasks
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
4 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
概要
変更確認方法
bug/review-of-event_comment_countをローカルに取り込むkomagataのアカウントでログインする。下記に箇条書きにて変更した箇所を記載します。
※コメントがない箇所はコメント数は表示されません。（以下同様です）
サイドカラムの箇所にもコメント数（直近の日報と提出物）が記載されています。
marumarushain1に変更しお知らせにコメントをしてくださいadvijirouに変更し登録情報変更から所属企業情報を「MaruMaru Inc」にして更新してくださいmarumarushain1を見つけコメント数が出ているか確認するScreenshot
変更前
2026-06-01.20.50.46.mov
変更後
2026-06-01.21.15.21.mov
Review App
Summary by CodeRabbit
Flakewatch Report