Conversation
- monsterQueryService.FetchList で query.Where() の戻り値を query に 再代入していなかったため WHERE 条件が無視されていた問題を修正 - GORM v2 では db.Where() は新しい *gorm.DB を返すため query = query.Where(...) が必要 - total カウントにも同じ WHERE 条件を適用し、フィルタ後の件数を正しく返すよう修正 - テストの wantTotal をフィルタ後の正しい期待値に更新 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Code Metrics Report
Code coverage of files in pull request scope (92.2%)
Reported by octocov |
There was a problem hiding this comment.
Pull request overview
GORM v2 の Where() が新しい *gorm.DB を返す仕様により、属性検索などの WHERE 条件がクエリに適用されず /v1/monsters?UsageElement=fire 等でデータが返らない不具合を修正するPRです。
Changes:
FetchListの WHERE 条件適用時にquery = query.Where(...)と再代入するよう修正total件数のカウントにも同一の WHERE 条件を適用するよう修正- テストの
wantTotalをフィルタ後の件数に合わせて更新
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/database/mysql/monsterQueryService.go | WHERE条件の適用漏れ修正、および total カウントに同一条件を適用 |
| internal/database/mysql/monsterQueryService_test.go | フィルタ適用後の Total 期待値に合わせてテストを更新 |
|
|
||
| if id != "" { | ||
| query.Where("monster_id = ? ", id) | ||
| query = query.Where("monster_id = ? ", id) |
There was a problem hiding this comment.
Where("monster_id = ? ", id) has an extra trailing space in the SQL fragment. It’s harmless for MySQL, but it’s inconsistent with the other queries in this function and makes it easier to miss subtle differences—please remove the trailing space for consistency.
| query = query.Where("monster_id = ? ", id) | |
| query = query.Where("monster_id = ?", id) |
| } else if where_clade != "" { | ||
| countQuery = countQuery.Where(where_clade, whereArgs...) | ||
| } | ||
| countQuery.Count(&total) |
There was a problem hiding this comment.
The result of countQuery.Count(&total) is not checked for errors. If the COUNT query fails (e.g., bad SQL, connection issues), the handler will still return a response with an incorrect total. Please capture the returned *gorm.DB, check Error, and return the error like the main Find query does.
| countQuery.Count(&total) | |
| countResult := countQuery.Count(&total) | |
| if countResult.Error != nil { | |
| return nil, countResult.Error | |
| } |
close #191
変更内容
monsterQueryService.FetchListでquery.Where()の戻り値をqueryに再代入していなかったため、WHERE 条件が無視されていた問題を修正totalカウントにも同じ WHERE 条件を適用し、フィルタ後の正確な件数を返すよう修正wantTotalをフィルタ後の正しい期待値に更新変更の背景・目的
/v1/monsters?UsageElement=fireで検索すると{"total":333}しか返らない問題db.Where(...)はレシーバーを変更せず新しい*gorm.DBを返すため、query = query.Where(...)と再代入しなければ WHERE 条件が失われるテスト結果
Test_monsterQueryService_FetchList全12ケース PASS）Closes #191