Skip to content

fix: shift_usecase の Find→Scan 連鎖でのエラー握り潰しを解消#378

Merged
taminororo merged 2 commits into
developfrom
refactor/kanba/369-shift-find-scan
Jun 24, 2026
Merged

fix: shift_usecase の Find→Scan 連鎖でのエラー握り潰しを解消#378
taminororo merged 2 commits into
developfrom
refactor/kanba/369-shift-find-scan

Conversation

@taminororo

@taminororo taminororo commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

概要

shift_usecase.go の4関数で、Find→Scan が連鎖する箇所のエラーが握り潰されていた(ineffassign 12件)。各 Find/Scan の直後に if err != nil { return } を追加して解消した。

変更した関数

関数 ineffassign 内容
GetUsersByShift 7件 year/date/time/weather の Find→Scan の中間 err が握り潰されていた
GetShiftAdminByID 1件 Find の err が Scan で上書きされていた
CreateShiftAdmin 2件 Create の err も握り潰されていた(本物のバグ: 後述)
UpdateShiftAdmin 2件 Find の err が Scan で上書きされていた

CreateShiftAdmin の本物のバグ

修正前:

err := u.rep.Create(c, ...)          // Create の err
row, err := u.rep.FindLatestRecord(c) // ← Create の err を上書き(握り潰し)

Create が失敗しても FindLatestRecord に進み、別ユーザーの最新シフト行を「自分の作成結果」として返していた。修正後は Create 失敗を呼び出し側に正しく伝播する。

安全性確認

  • 全 Scan 箇所で Scan列・SQLのSELECT列・DBスキーマが一致することを静的解析で確認(列ズレによる常時失敗バグは無し)
  • 正常系は if err != nil { return } が no-op なので挙動変化なし
  • GetUsersByShift の ErrNoRows 懸念は getBeforeMembers/getAfterMembers の2段ガードで遮断済み

既存の lint 警告について

golangci-lint run に5件の既存警告が残るが、いずれも今回の変更とは無関係のファイル(rescue_unified_usecase.go / mail_auth_usecase.go / user_usecase.go)。CI は only-new-issues: true のため影響なし。

Closes #369

Summary by CodeRabbit

  • Bug Fixes
    • シフト関連の取得・作成・更新処理で、データベースの取得失敗や読み取り失敗をその場で正しく返すようになりました。
    • これにより、エラー発生時の挙動がより安定し、途中で不正な結果が返る可能性が減りました。

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@taminororo, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 36 minutes and 53 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 632fa3f7-4474-458b-8a34-2e7ebab239a8

📥 Commits

Reviewing files that changed from the base of the PR and between 152829b and b78e59e.

📒 Files selected for processing (1)
  • api/lib/usecase/shift_usecase.go
📝 Walkthrough

Walkthrough

api/lib/usecase/shift_usecase.go の4関数(GetUsersByShiftGetShiftAdminByIDCreateShiftAdminUpdateShiftAdmin)において、Find 呼び出しと row.Scan 呼び出しの各段にエラーチェックと即時 return ガードが追加された。

Changes

shift_usecase.go Find→Scan エラーハンドリング強化

Layer / File(s) Summary
GetUsersByShift の Find/Scan エラーガード
api/lib/usecase/shift_usecase.go
yearRep.FinddateRep.FindtimeRep.FindweatherRep.Find それぞれの err 検査と各 row.Scanerr 検査を逐次追加し、失敗時に即 shiftUsers を返すよう変更された。
GetShiftAdminByID / CreateShiftAdmin / UpdateShiftAdmin のガード追加
api/lib/usecase/shift_usecase.go
GetShiftAdminByIDa.rep.Find エラー即時 return、CreateShiftAdminu.rep.Create および FindLatestRecord エラー即時 return、UpdateShiftAdmin の初回・再取得 u.rep.Find エラー即時 return がそれぞれ追加された。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

  • #369: このPRは issue #369 が挙げた4関数(GetUsersByShiftGetShiftAdminByIDCreateShiftAdminUpdateShiftAdmin)の連鎖 Find→Scan における中間エラー握り潰しを修正しており、ineffassign 件数削減と早期 return 追加の目的に直接対応している。

Possibly related PRs

  • NUTFes/SeeFT#362: shift_usecase.goUpdateShiftAdmin および DB 書き込みエラー伝播を修正しており、本PRと同一ファイルの同一エラーハンドリング経路を対象としている。

Poem

🐇 ぴょんぴょんと、エラーを早めに返すんだ
Scan の前に Find の失敗を見逃さず
後段に流れていた霧が晴れた
4つの関数、今日から安心
うさぎの目は誤魔化されないよ 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning 4関数の中間err修正は一致していますが、#369の要件であるcharacterizationテスト追加とグローバル変数削除が見当たりません。 対象4関数のcharacterizationテストを追加し、shift_usecase.go の TaskID/UserID/YearID/DateID/TimeID/WeatherID/PlaceID を削除してください。
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed PRの主変更であるshift_usecase.goのFind→Scan連鎖エラー修正を端的に表しており、内容と一致しています。
Description check ✅ Passed 概要、対象4関数の変更理由、CreateShiftAdminのバグ説明、issue参照があり、必須でない項目の欠落のみです。
Out of Scope Changes check ✅ Passed 変更はshift_usecase.go内のエラーハンドリング整理に限られ、要件外の機能追加や無関係な改修は確認できません。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/kanba/369-shift-find-scan

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

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
api/lib/usecase/shift_usecase.go (1)

199-206: 🗄️ Data Integrity & Integration | 🔵 Trivial

CreateShiftAdminFindLatestRecord ではなく CreateAndReturnID を使いたい。 Create 後に最新1件を再取得する方式だと、同時作成時に別リクエストの行を返す可能性があります。RETURNING id で対象レコードを直接取得する方が安全です。

🤖 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 `@api/lib/usecase/shift_usecase.go` around lines 199 - 206, CreateShiftAdmin
currently calls u.rep.Create and then u.rep.FindLatestRecord, which can return
the wrong row under concurrent inserts. Update the CreateShiftAdmin flow in
shift_usecase.go to use u.rep.CreateAndReturnID instead of re-querying the
latest record, and propagate the returned row/id directly from that call. Keep
the existing error handling around the repository call, and remove the
FindLatestRecord dependency from this path.
🤖 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 `@api/lib/usecase/shift_usecase.go`:
- Around line 199-206: CreateShiftAdmin currently calls u.rep.Create and then
u.rep.FindLatestRecord, which can return the wrong row under concurrent inserts.
Update the CreateShiftAdmin flow in shift_usecase.go to use
u.rep.CreateAndReturnID instead of re-querying the latest record, and propagate
the returned row/id directly from that call. Keep the existing error handling
around the repository call, and remove the FindLatestRecord dependency from this
path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8982f65f-3413-44b6-8f80-5a53a7ddd0cf

📥 Commits

Reviewing files that changed from the base of the PR and between f7789a3 and 152829b.

📒 Files selected for processing (1)
  • api/lib/usecase/shift_usecase.go

@taminororo taminororo merged commit 7ac5f19 into develop Jun 24, 2026
2 checks passed
@taminororo taminororo deleted the refactor/kanba/369-shift-find-scan branch June 24, 2026 17:29
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.

[Go static/refactor] shift_usecase.go の連鎖 Find→Scan リファクタ(ineffassign 12件・対象4関数)

1 participant