Skip to content

Fix: 新規登録ユーザーに登録前のお知らせが未読表示される不具合を修正 - #327

Merged
ippei-shimizu merged 2 commits into
stgfrom
fix/448-management-notice-new-user
Jul 28, 2026
Merged

Fix: 新規登録ユーザーに登録前のお知らせが未読表示される不具合を修正#327
ippei-shimizu merged 2 commits into
stgfrom
fix/448-management-notice-new-user

Conversation

@ippei-shimizu

Copy link
Copy Markdown
Owner

Summary

  • 新規登録ユーザーが、登録前(過去)に公開済みの運営からのお知らせ(ManagementNotice)を全て未読として表示・カウントしてしまう不具合を修正
  • Userbefore_create フックを追加し、last_management_notice_read_at をユーザー登録時刻で初期化する。これにより登録前のお知らせは既読扱いになり、登録後に公開されたお知らせのみ未読として扱われる
  • 既存ユーザーには影響しない(before_create のため新規作成時のみ発火)
  • v1スキーマスナップショット(spec/golden_v1/users_show.json)の last_management_notice_read_atnull 固定ではなくなったため更新

Fixes

Fixes ippei-shimizu/buzzbase#448

Test plan

  • spec/models/user_spec.rblast_management_notice_read_at 初期化のモデルスペックを追加
  • spec/requests/api/v1/notifications_management_notice_spec.rb を新規作成し、登録前/登録後のお知らせで index・count の挙動を確認
  • bundle exec rspec (関連spec一式)全て成功
  • bundle exec rubocop 変更ファイルすべて指摘なし

🤖 Generated with Claude Code

登録時点で last_management_notice_read_at を初期化していなかったため、
新規ユーザーは登録前に公開済みの運営からのお知らせも全て未読としてカウント・
表示されていた。ユーザー作成時に last_management_notice_read_at を登録時刻で
初期化し、登録前のお知らせは既読扱いになるようにする。

v1スキーマスナップショット(users_show.json)もフィールドの null 許容が
変わったため合わせて更新する。
@ippei-shimizu ippei-shimizu self-assigned this Jul 28, 2026
@ippei-shimizu

Copy link
Copy Markdown
Owner Author

@claude
コードレビュー

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown

Claude finished @ippei-shimizu's task in 3m 19s —— View job


コードレビュー

タスクリスト

  • PRの変更内容を取得・分析
  • app/models/user.rb のレビュー
  • テストコードのレビュー
  • golden_v1 スナップショット変更のレビュー
  • レビューコメントを投稿

全体評価

修正内容は妥当です。before_createlast_management_notice_read_at を登録時刻で初期化することで、Api::V1::NotificationsController#index / #count の「published_at <= last_management_notice_read_at」判定が意図通り機能し、登録前のお知らせが未読扱いされなくなります。既存ユーザーは before_create のため影響を受けず、||= により明示的に値を渡すケース(シード/管理系)も壊していません。コメント(app/models/user.rb:73)も CLAUDE.md の規約通り WHY を簡潔に書けています。

気になった点

1. 新規リクエストスペックの published_at: 指定が実質無効になっている(要確認)

spec/requests/api/v1/notifications_management_notice_spec.rb:6, 21, 36, 47

create(:management_notice, :published, published_at: 3.days.ago)
create(:management_notice, :published, published_at: 1.minute.from_now)

のように明示的な published_at を渡していますが、ManagementNotice#set_published_atapp/models/management_notice.rb:17-19)は

def set_published_at
  self.published_at = Time.current if status_changed? && published?
end

という before_save コールバックで、新規レコードは status のデフォルトが draftdb/schema.rb:258)なので status: :published を渡した時点で常に status_changed?true になります。つまり 渡した published_at は保存時に必ず Time.current で上書きされ、実際には「3日前」「1分後」いずれのケースも作成時刻(≒ほぼ現在時刻)になります。

現状のテストが green なのは、old_notice はユーザー作成より前に、new_notice はユーザー作成より後に生成しているため、実際の published_at の前後関係(お知らせ作成 → ユーザー作成 → published_at ≤ read_at)がたまたま意図通りになっているからです。つまり「作成順序」がテストの成否を決めており、指定した日時の値そのものは検証に寄与していません。可読性・意図の明確化という観点で、以下のような修正を推奨します。

old_notice = create(:management_notice, :published)
old_notice.update_column(:published_at, 3.days.ago) # set_published_at の上書きを回避

または travel_to で時間を固定してから作成順序自体をテスト対象にする、などです。

これは今回のPRで新規に発生した問題ではなく既存の ManagementNotice の挙動(他のspecでもこのパターンの使用例なし)ですが、追加された新しいテストが「日付」ではなく「作成順序」に依存していることは意図せぬ回帰(例: 将来的にお知らせ作成とユーザー作成の順序が変わるリファクタ)を見逃す可能性があるため、テストの意図をコード上で明確にしておくと安全です。

Fix this →

2. golden_v1 スナップショット更新

last_management_notice_read_atnullString に変わったのは今回の修正で新規ユーザーに必ず値が入るようになった結果であり、想定通りの更新です。問題ありません。

3. モデルスペックspec/models/user_spec.rb:428-441

初期化されるケース/明示的な値を上書きしないケースの両方をカバーしており妥当です。

実行できなかったこと

サンドボックス環境の制約により bundle exec rspec / bundle exec rubocop をこの場で実行できませんでした(コマンド実行が承認待ちで止まるため)。PR descriptionでは両方成功と記載されているので、上記1.の指摘のみ実装側で妥当性をご確認いただければと思います。

…atで上書きされ検証に効かない問題を修正

ManagementNotice#set_published_at が新規作成時に published_at を
Time.current で上書きするため、create時に渡した published_at は無視され
テストは作成順序にのみ依存していた。update_column でコールバックを回避し
published_at を明示的に固定することで、日時ベースの判定を実際に検証する。
@ippei-shimizu
ippei-shimizu merged commit ca78ed9 into stg Jul 28, 2026
2 checks passed
@ippei-shimizu
ippei-shimizu deleted the fix/448-management-notice-new-user branch July 28, 2026 14:40
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.

Bug: 新規登録ユーザーに登録前のお知らせが未読表示されてしまう

1 participant