Conversation
|
Migrate lint ✅ Lint output |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Migrate lint ✅ Lint output |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1470 +/- ##
==========================================
- Coverage 52.54% 51.42% -1.13%
==========================================
Files 142 143 +1
Lines 10282 10506 +224
==========================================
Hits 5403 5403
- Misses 4559 4783 +224
Partials 320 320 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
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:
📝 WalkthroughWalkthroughゲームフィードバック機能の追加。DBスキーマ(4テーブル)、マイグレーション、OpenAPI仕様、設計ドキュメント、GORMスキーマ、ハンドラー骨組み、生成されたOpenAPIコードを導入し、APIエンドポイント群と型を追加しています(実装は大半が未実装スタブ)。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Launcher as Launcher (Client)
participant API as API / Handler
participant Repo as Repository (GORM)
participant DB as Database
Launcher->>API: POST /games/{gameID}/feedbacks (answers, comment, gameVersionID)
API->>API: validate request (OpenAPI types / discriminator)
API->>Repo: Create GameFeedback + Answers (transaction)
Repo->>DB: INSERT game_feedbacks
DB-->>Repo: OK (id)
Repo->>DB: INSERT game_feedback_answers (multiple)
DB-->>Repo: OK
Repo-->>API: success
API-->>Launcher: 201 Created (GameFeedback)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/handler/v2/openapi/openapi.gen.go (1)
1-4: 生成ファイルの変更はdocs/openapi/v2.yaml起点で再生成に統一してください。このファイルは生成物なので、手編集が混ざると次回生成で差分が失われます。仕様変更は
docs/openapi/v2.yaml、実装はsrc/handler/v2/*に寄せる運用を維持するのが安全です。As per coding guidelines: Do not directly edit generated OpenAPI code in
src/handler/v2/openapi. Hand-write implementation code insrc/handler/v2/*instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/handler/v2/openapi/openapi.gen.go` around lines 1 - 4, This file is a generated artifact in package openapi (openapi.gen.go) and must not be hand-edited; revert any manual changes in the generated package openapi, move any custom logic into your handwritten handler package (the v2 handlers), and re-generate the OpenAPI client by updating the canonical spec (docs/openapi/v2.yaml) and running the oapi-codegen generator (v2.5.0) to produce a fresh openapi package; ensure all implementation code lives in the non-generated handler code rather than the generated package.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/feedback-api-design.md`:
- Around line 39-72: The markdown code fences containing the schema examples
(the blocks that start with "game_feedback_configs (フィードバック設定 — ゲーム単位)",
"feedback_questions", "game_feedbacks", "game_feedback_answers" and the other
block around lines 186-199) lack a language tag and trigger markdownlint MD040;
update each triple-backtick fence to include a language specifier (e.g.,
```text) at the opening fence and keep the closing fence as ``` so CI warnings
are resolved. Ensure you modify every matching fenced block in the file
(including the block shown in the diff and the block referenced at 186-199).
In `@docs/openapi/v2.yaml`:
- Around line 4248-4260: The OpenAPI schema for the feedback input currently
allows an empty answers array because the answers property (items: $ref:
'#/components/schemas/FeedbackAnswerInput') lacks a minItems constraint; update
the answers schema to include "minItems: 1" (and optionally "uniqueItems: true"
if duplicates should be disallowed) so at least one FeedbackAnswerInput is
required while keeping the existing required list and additionalProperties:
false.
- Around line 1621-1632: The POST /editions/{editionID}/games/{gameID}/feedbacks
operation (operationId: postGameFeedback) currently uses only EditionAuth;
replace it with EditionIDAuth so the token’s edition is validated against the
path param. Locate the security block under the postGameFeedback operation and
change the entry from "EditionAuth: []" to "EditionIDAuth: []" (keeping any
other security entries intact) to enforce edition-id matching like the GET
/editions/{editionID}/games endpoint.
In `@migrations/20260225125932_create_game_feedbacks.sql`:
- Around line 29-39: Add a uniqueness constraint to prevent duplicate answers
per question within the same feedback by adding a UNIQUE index/constraint on
(feedback_id, question_id) to the game_feedback_answers table; update the CREATE
TABLE for game_feedback_answers to include something like a UNIQUE KEY
(feedback_id, question_id) (e.g., name it
uniq_game_feedback_answers_feedback_question) alongside the existing indexes and
foreign keys so the DB enforces one answer per (feedback_id, question_id).
- Around line 16-39: Add a game_id column to game_feedbacks and enforce it
references the games table (CONSTRAINT on game_feedbacks.game_id -> games.id),
backfill/populate it from v2_game_versions via game_version_id, and ensure it
stays in sync (e.g., trigger on INSERT/UPDATE of game_feedbacks to validate
game_id matches v2_game_versions.game_id). Then add a DB-level check to prevent
cross-game answers by validating in game_feedback_answers (e.g., trigger on
INSERT/UPDATE) that the referenced feedback (game_feedbacks.id) and the
referenced question (feedback_questions.id) share the same game_id; use the
table names and columns from the diff: game_feedbacks.game_id,
game_feedbacks.game_version_id, game_feedback_answers.feedback_id/question_id,
and feedback_questions.game_id to locate where to implement these changes.
In `@src/handler/v2/api.go`:
- Line 31: API 構造体の *GameCreator フィールドが NewAPI で初期化されていないので、NewAPI 関数内で
GameFeedback と同じパターンを使って *GameCreator をセットしてください;具体的には NewAPI の初期化ブロックで既存の
GameFeedback 初期化コードを参照し、必要な引数を渡して GameCreator を生成し API
の対応フィールドに代入するように修正してください(参照シンボル: API, NewAPI, GameFeedback, GameCreator)。
- Line 32: API's dependency injection is incomplete: the embedded pointers
GameFeedback and GameCreator in type API are not provided by NewAPI and their
constructors (NewGameFeedback, NewGameCreator) and providers are not registered
in the Wire set, so they remain nil; fix by adding GameFeedback and GameCreator
parameters to NewAPI's signature and assign them to the returned API instance,
implement simple constructors NewGameFeedback and NewGameCreator that return the
stub structs, and register v2.NewGameFeedback and v2.NewGameCreator in
handlerV2Set in src/wire/handler.go so Wire can wire them automatically.
In `@src/repository/gorm2/schema/v2.go`:
- Around line 292-293: schema定義とマイグレーションで型不一致が発生しているので、構造体内の QuestionOrder と
Answer フィールドの Go 型を int から int64 に変更し(該当フィールド名: QuestionOrder, Answer)、対応する GORM
タグの型指定も "type:int" から "type:bigint" に合わせて修正してマイグレーションの bigint と整合させてください。
---
Nitpick comments:
In `@src/handler/v2/openapi/openapi.gen.go`:
- Around line 1-4: This file is a generated artifact in package openapi
(openapi.gen.go) and must not be hand-edited; revert any manual changes in the
generated package openapi, move any custom logic into your handwritten handler
package (the v2 handlers), and re-generate the OpenAPI client by updating the
canonical spec (docs/openapi/v2.yaml) and running the oapi-codegen generator
(v2.5.0) to produce a fresh openapi package; ensure all implementation code
lives in the non-generated handler code rather than the generated package.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88030bac-afcd-40b0-80df-03731a1d921f
⛔ Files ignored due to path filters (9)
docs/db_schema/editions.svgis excluded by!**/*.svgdocs/db_schema/feedback_questions.svgis excluded by!**/*.svgdocs/db_schema/game_feedback_answers.svgis excluded by!**/*.svgdocs/db_schema/game_feedback_configs.svgis excluded by!**/*.svgdocs/db_schema/game_feedbacks.svgis excluded by!**/*.svgdocs/db_schema/games.svgis excluded by!**/*.svgdocs/db_schema/schema.svgis excluded by!**/*.svgdocs/db_schema/v2_game_versions.svgis excluded by!**/*.svgmigrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
docs/db_schema/README.mddocs/db_schema/editions.mddocs/db_schema/feedback_questions.mddocs/db_schema/game_feedback_answers.mddocs/db_schema/game_feedback_configs.mddocs/db_schema/game_feedbacks.mddocs/db_schema/games.mddocs/db_schema/v2_game_versions.mddocs/design/feedback-api-design.mddocs/openapi/v2.yamlmigrations/20260225125932_create_game_feedbacks.sqlsrc/handler/v2/api.gosrc/handler/v2/game_feedback.gosrc/handler/v2/openapi/openapi.gen.gosrc/repository/gorm2/schema/v2.go
| ``` | ||
| game_feedback_configs (フィードバック設定 — ゲーム単位) | ||
| ├── game_id: UUID (PK, FK -> games) | ||
| ├── enabled: BOOLEAN -- フィードバック機能のon/off | ||
| └── [FK] game -> GameTable2 | ||
|
|
||
| feedback_questions (質問マスタ — ゲーム単位) | ||
| ├── id: UUID (PK) | ||
| ├── game_id: UUID (FK -> games) -- ゲーム | ||
| ├── question_text: VARCHAR(256) -- 質問文 | ||
| ├── answer_type: TINYINT -- 回答形式 (0=yesNo, 1=fiveScale) | ||
| ├── question_order: INT -- 表示順序 | ||
| ├── created_at: DATETIME | ||
| ├── archived_at: DATETIME NULL -- アーカイブ日時(過去回答は取得可、新規回答対象外) | ||
| ├── deleted_at: DATETIME NULL -- 論理削除(回答も取得不可) | ||
| └── [FK] game -> GameTable2 | ||
|
|
||
| game_feedbacks (フィードバック本体) | ||
| ├── id: UUID (PK) | ||
| ├── edition_id: UUID (FK -> editions) | ||
| ├── game_version_id: UUID (FK -> v2_game_versions) | ||
| ├── comment: TEXT NULL -- 自由記述欄 | ||
| ├── created_at: DATETIME | ||
| ├── [FK] edition -> EditionTable | ||
| └── [FK] game_version -> GameVersionTable2 | ||
|
|
||
| game_feedback_answers (質問ごとの回答) | ||
| ├── id: UUID (PK) | ||
| ├── feedback_id: UUID (FK -> game_feedbacks) | ||
| ├── question_id: UUID (FK -> feedback_questions) | ||
| ├── answer: INT -- 回答値 (yesNo: 0/1, fiveScale: 1-5) | ||
| ├── [FK] feedback -> GameFeedbackTable | ||
| └── [FK] question -> FeedbackQuestionTable | ||
| ``` |
There was a problem hiding this comment.
コードフェンスに言語指定がなく、markdownlint(MD040)警告が出ます。
CIノイズを減らすため、コードブロックに言語(例: text)を付けてください。
📝 修正案
-```
+```text
game_feedback_configs (フィードバック設定 — ゲーム単位)
...
-```
+```
-```
+```text
有効 (active)
...
-```
+```Also applies to: 186-199
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design/feedback-api-design.md` around lines 39 - 72, The markdown code
fences containing the schema examples (the blocks that start with
"game_feedback_configs (フィードバック設定 — ゲーム単位)", "feedback_questions",
"game_feedbacks", "game_feedback_answers" and the other block around lines
186-199) lack a language tag and trigger markdownlint MD040; update each
triple-backtick fence to include a language specifier (e.g., ```text) at the
opening fence and keep the closing fence as ``` so CI warnings are resolved.
Ensure you modify every matching fenced block in the file (including the block
shown in the diff and the block referenced at 186-199).
| answers: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/FeedbackAnswerInput' | ||
| description: '質問に対する回答の配列' | ||
| comment: | ||
| type: string | ||
| maxLength: 2000 | ||
| description: '自由記述コメント(任意)' | ||
| required: | ||
| - gameVersionID | ||
| - answers | ||
| additionalProperties: false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ファイルの存在確認とサイズ確認
ls -la docs/openapi/v2.yaml 2>/dev/null || echo "File not found"
wc -l docs/openapi/v2.yaml 2>/dev/null || echo "Cannot get line count"Repository: traPtitech/trap-collection-server
Length of output: 174
🏁 Script executed:
#!/bin/bash
# 指定行を確認
if [ -f "docs/openapi/v2.yaml" ]; then
echo "=== Lines 4248-4260 ==="
sed -n '4248,4260p' docs/openapi/v2.yaml
echo ""
echo "=== Context: Lines 4240-4270 ==="
sed -n '4240,4270p' docs/openapi/v2.yaml
fiRepository: traPtitech/trap-collection-server
Length of output: 1255
🏁 Script executed:
#!/bin/bash
# ハンドラー実装を確認
find src/handler -type f -name "*.go" | head -20Repository: traPtitech/trap-collection-server
Length of output: 659
🏁 Script executed:
#!/bin/bash
# PostGameFeedbackRequest の使用箇所を確認
rg "PostGameFeedbackRequest" --type goRepository: traPtitech/trap-collection-server
Length of output: 361
🏁 Script executed:
#!/bin/bash
# PostGameFeedback ハンドラー実装を探索
rg "PostGameFeedback" src/handler/v2/ --type go -A 5 | head -100Repository: traPtitech/trap-collection-server
Length of output: 4243
🏁 Script executed:
#!/bin/bash
# ハンドラー内容確認
if [ -f "src/handler/v2/game.go" ]; then
wc -l src/handler/v2/game.go
fiRepository: traPtitech/trap-collection-server
Length of output: 103
🏁 Script executed:
#!/bin/bash
# game_feedback.go の内容全体を確認
cat -n src/handler/v2/game_feedback.goRepository: traPtitech/trap-collection-server
Length of output: 1701
answers配列が空でも通るため、「回答必須」の仕様を満たせません。
answers は required ですが minItems がないので空配列を許可してしまいます。少なくとも1件は要求する制約を追加してください。
修正案
PostGameFeedbackRequest:
type: object
properties:
gameVersionID:
$ref: '#/components/schemas/GameVersionID'
answers:
type: array
+ minItems: 1
items:
$ref: '#/components/schemas/FeedbackAnswerInput'
description: '質問に対する回答の配列'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| answers: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/FeedbackAnswerInput' | |
| description: '質問に対する回答の配列' | |
| comment: | |
| type: string | |
| maxLength: 2000 | |
| description: '自由記述コメント(任意)' | |
| required: | |
| - gameVersionID | |
| - answers | |
| additionalProperties: false | |
| answers: | |
| type: array | |
| minItems: 1 | |
| items: | |
| $ref: '#/components/schemas/FeedbackAnswerInput' | |
| description: '質問に対する回答の配列' | |
| comment: | |
| type: string | |
| maxLength: 2000 | |
| description: '自由記述コメント(任意)' | |
| required: | |
| - gameVersionID | |
| - answers | |
| additionalProperties: false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/openapi/v2.yaml` around lines 4248 - 4260, The OpenAPI schema for the
feedback input currently allows an empty answers array because the answers
property (items: $ref: '#/components/schemas/FeedbackAnswerInput') lacks a
minItems constraint; update the answers schema to include "minItems: 1" (and
optionally "uniqueItems: true" if duplicates should be disallowed) so at least
one FeedbackAnswerInput is required while keeping the existing required list and
additionalProperties: false.
| @@ -29,6 +29,7 @@ type API struct { | |||
| *GameVideo | |||
| *GamePlayLog | |||
| *GameCreator | |||
There was a problem hiding this comment.
GameCreator も同様に初期化されていません。
*GameCreator フィールドも API 構造体に存在しますが、NewAPI で初期化されていません。GameFeedback と同じパターンで修正が必要です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/handler/v2/api.go` at line 31, API 構造体の *GameCreator フィールドが NewAPI
で初期化されていないので、NewAPI 関数内で GameFeedback と同じパターンを使って *GameCreator をセットしてください;具体的には
NewAPI の初期化ブロックで既存の GameFeedback 初期化コードを参照し、必要な引数を渡して GameCreator を生成し API
の対応フィールドに代入するように修正してください(参照シンボル: API, NewAPI, GameFeedback, GameCreator)。
There was a problem hiding this comment.
@mathsuky 了解しました。別PRでの対応を確認しました。引き続きよろしくお願いします!
| *GameVideo | ||
| *GamePlayLog | ||
| *GameCreator | ||
| *GameFeedback |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the API struct definition around line 32
sed -n '25,50p' src/handler/v2/api.goRepository: traPtitech/trap-collection-server
Length of output: 461
🏁 Script executed:
# Check the NewAPI function and return statement
sed -n '34,80p' src/handler/v2/api.goRepository: traPtitech/trap-collection-server
Length of output: 973
🏁 Script executed:
# Search for wire handler configuration
rg -n 'handlerV2Set|NewGameFeedback' src/wire/ -A 3Repository: traPtitech/trap-collection-server
Length of output: 406
🏁 Script executed:
# Check if NewGameFeedback constructor exists
rg -n 'func NewGameFeedback' src/handler/v2/Repository: traPtitech/trap-collection-server
Length of output: 59
🏁 Script executed:
# Also check GameCreator as mentioned
rg -n 'GameCreator' src/handler/v2/api.goRepository: traPtitech/trap-collection-server
Length of output: 93
🏁 Script executed:
# Check the full handlerV2Set definition in wire/handler.go
sed -n '39,120p' src/wire/handler.goRepository: traPtitech/trap-collection-server
Length of output: 420
🏁 Script executed:
# Check if GameCreator has the same issue - look for NewGameCreator
rg -n 'func NewGameCreator' src/handler/v2/Repository: traPtitech/trap-collection-server
Length of output: 59
🏁 Script executed:
# Check what providers ARE defined in the wire set - look for v2.New* patterns
rg -n 'v2\.New' src/wire/handler.goRepository: traPtitech/trap-collection-server
Length of output: 440
🏁 Script executed:
# Check if NewGameFeedback constructor exists
rg -n 'func NewGameFeedback' src/handler/v2/Repository: traPtitech/trap-collection-server
Length of output: 59
🏁 Script executed:
# Check what file GameFeedback is defined in
rg -n 'type GameFeedback struct' src/handler/v2/Repository: traPtitech/trap-collection-server
Length of output: 207
🏁 Script executed:
# Look for any feedback handler file
fd -t f 'feedback' src/handler/v2/Repository: traPtitech/trap-collection-server
Length of output: 108
🏁 Script executed:
# Check the full game_feedback.go file
cat -n src/handler/v2/game_feedback.goRepository: traPtitech/trap-collection-server
Length of output: 1701
🏁 Script executed:
# Check if GameCreator is similar to GameFeedback
cat -n src/handler/v2/game_creator.go | head -50Repository: traPtitech/trap-collection-server
Length of output: 1016
GameFeedback と GameCreator が NewAPI で初期化されていないため、依存性注入が不完全です。
*GameFeedback と *GameCreator が API 構造体に埋め込まれていますが、NewAPI 関数のパラメータに含まれておらず、return 文でも初期化されていません。さらに src/wire/handler.go の handlerV2Set に v2.NewGameFeedback と v2.NewGameCreator プロバイダーも登録されていません。
加えて、NewGameFeedback コンストラクタが存在しないため、Wire による自動注入も不可能です。GameCreator についても同様です。
これらは現在スタブ実装(空の構造体で NotImplemented を返す)ですが、他のハンドラーと同じ方式で初期化される必要があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/handler/v2/api.go` at line 32, API's dependency injection is incomplete:
the embedded pointers GameFeedback and GameCreator in type API are not provided
by NewAPI and their constructors (NewGameFeedback, NewGameCreator) and providers
are not registered in the Wire set, so they remain nil; fix by adding
GameFeedback and GameCreator parameters to NewAPI's signature and assign them to
the returned API instance, implement simple constructors NewGameFeedback and
NewGameCreator that return the stub structs, and register v2.NewGameFeedback and
v2.NewGameCreator in handlerV2Set in src/wire/handler.go so Wire can wire them
automatically.
| QuestionOrder int `gorm:"type:int;not null"` | ||
| CreatedAt time.Time `gorm:"type:datetime;not null;default:CURRENT_TIMESTAMP"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the GORM schema file
fd -t f "v2.go" src/repository/gorm2/schema/Repository: traPtitech/trap-collection-server
Length of output: 110
🏁 Script executed:
# Examine the schema file content around the specified lines
cat -n src/repository/gorm2/schema/v2.go | sed -n '280,330p'Repository: traPtitech/trap-collection-server
Length of output: 2613
🏁 Script executed:
# Find and examine the migration file
fd -t f "*create_game_feedbacks*" migrations/Repository: traPtitech/trap-collection-server
Length of output: 379
🏁 Script executed:
# Check the migration file content for field types
cat -n migrations/20260225125932_create_game_feedbacks.sql 2>/dev/null || find migrations -name "*game_feedback*" -o -name "*feedbacks*" | head -5Repository: traPtitech/trap-collection-server
Length of output: 2824
GORM定義とマイグレーション定義で型が不一致です。
question_order・answer がマイグレーションでは bigint、ここでは int になっており、スキーマドリフトの原因になります。Go の int は 32 ビット、MySQL の bigint は 64 ビットのため、マイグレーションに合わせて int64 に変更してください。
🔧 修正案
type FeedbackQuestionTable struct {
ID uuid.UUID `gorm:"type:varchar(36);not null;primaryKey"`
GameID uuid.UUID `gorm:"type:varchar(36);not null;index"`
QuestionText string `gorm:"type:varchar(256);not null"`
AnswerType int `gorm:"type:tinyint;not null"`
- QuestionOrder int `gorm:"type:int;not null"`
+ QuestionOrder int64 `gorm:"type:bigint;not null"`
CreatedAt time.Time `gorm:"type:datetime;not null;default:CURRENT_TIMESTAMP"`
ArchivedAt sql.NullTime `gorm:"type:DATETIME NULL;default:NULL"`
DeletedAt gorm.DeletedAt `gorm:"type:DATETIME NULL;default:NULL"`
Game GameTable2 `gorm:"foreignKey:GameID"`
}
type GameFeedbackAnswerTable struct {
ID uuid.UUID `gorm:"type:varchar(36);not null;primaryKey"`
FeedbackID uuid.UUID `gorm:"type:varchar(36);not null;index"`
QuestionID uuid.UUID `gorm:"type:varchar(36);not null;index"`
- Answer int `gorm:"type:int;not null"`
+ Answer int64 `gorm:"type:bigint;not null"`
Feedback GameFeedbackTable `gorm:"foreignKey:FeedbackID"`
Question FeedbackQuestionTable `gorm:"foreignKey:QuestionID"`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| QuestionOrder int `gorm:"type:int;not null"` | |
| CreatedAt time.Time `gorm:"type:datetime;not null;default:CURRENT_TIMESTAMP"` | |
| QuestionOrder int64 `gorm:"type:bigint;not null"` | |
| CreatedAt time.Time `gorm:"type:datetime;not null;default:CURRENT_TIMESTAMP"` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repository/gorm2/schema/v2.go` around lines 292 - 293,
schema定義とマイグレーションで型不一致が発生しているので、構造体内の QuestionOrder と Answer フィールドの Go 型を int から
int64 に変更し(該当フィールド名: QuestionOrder, Answer)、対応する GORM タグの型指定も "type:int" から
"type:bigint" に合わせて修正してマイグレーションの bigint と整合させてください。
|
この前の議論を踏まえて,
という形にしました。 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/openapi/v2.yaml (2)
4252-4256:⚠️ Potential issue | 🟠 Major
answersにminItems: 1を追加してください。Line 4252-4256 は required ですが空配列を許容してしまい、回答必須の要件を満たせません。
修正案
PostGameFeedbackRequest: type: object properties: @@ answers: type: array + minItems: 1 items: $ref: '#/components/schemas/FeedbackAnswerInput'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/v2.yaml` around lines 4252 - 4256, The 'answers' array schema currently allows empty arrays; update the OpenAPI schema for the property named "answers" (the array with items -> $ref: '#/components/schemas/FeedbackAnswerInput') to enforce at least one element by adding minItems: 1 to that schema so empty arrays are rejected and the required constraint behaves as intended.
1622-1633:⚠️ Potential issue | 🟠 Major
postGameFeedbackはEditionIDAuthを使ってください。Line 1632 が
EditionAuthのままだと、editionIDパスとの一致検証をAPI契約で担保できません。修正案
/editions/{editionID}/games/{gameID}/feedbacks: post: @@ security: - - EditionAuth: [] + - EditionIDAuth: []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/v2.yaml` around lines 1622 - 1633, The operationId postGameFeedback is using the wrong security scheme; change the security entry from EditionAuth to EditionIDAuth so the API contract can validate the editionID path parameter against the auth context; locate the postGameFeedback operation (security: EditionAuth) and replace that security item with EditionIDAuth to ensure editionID path matching is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/openapi/v2.yaml`:
- Around line 1567-1580: The OpenAPI operation putFeedbackQuestions currently
uses the GameMaintainerAuth security scheme which conflicts with the requirement
that only administrators may update questions; update the security entry for the
operationId putFeedbackQuestions to use the administrator-level security scheme
used elsewhere (e.g., AdminAuth or the project’s equivalent admin scheme)
instead of GameMaintainerAuth, ensuring the security array under the put
operation is replaced accordingly and matches the exact scheme name used by
other admin-protected endpoints.
---
Duplicate comments:
In `@docs/openapi/v2.yaml`:
- Around line 4252-4256: The 'answers' array schema currently allows empty
arrays; update the OpenAPI schema for the property named "answers" (the array
with items -> $ref: '#/components/schemas/FeedbackAnswerInput') to enforce at
least one element by adding minItems: 1 to that schema so empty arrays are
rejected and the required constraint behaves as intended.
- Around line 1622-1633: The operationId postGameFeedback is using the wrong
security scheme; change the security entry from EditionAuth to EditionIDAuth so
the API contract can validate the editionID path parameter against the auth
context; locate the postGameFeedback operation (security: EditionAuth) and
replace that security item with EditionIDAuth to ensure editionID path matching
is enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b97efe90-9210-4cb7-82ef-d2e6a92fc2cb
📒 Files selected for processing (2)
docs/openapi/v2.yamlsrc/handler/v2/openapi/openapi.gen.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
docs/openapi/v2.yaml (2)
1578-1579:⚠️ Potential issue | 🟠 Major質問更新権限が強すぎず、要件と不一致です。
putFeedbackQuestionsがGameMaintainerAuthだとゲーム側のメンテナが質問を変更できます。管理者設定前提の仕様に合わせるならAdminAuthにすべきです。修正案
security: - - GameMaintainerAuth: [] + - AdminAuth: []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/v2.yaml` around lines 1578 - 1579, The OpenAPI operation for putFeedbackQuestions is using GameMaintainerAuth which grants too-broad permissions; update the operation's security entry so that putFeedbackQuestions requires AdminAuth instead of GameMaintainerAuth, and verify the AdminAuth security scheme is defined in the document (adding it if missing) so the endpoint reflects admin-only access.
4247-4251:⚠️ Potential issue | 🟠 Major
answersが空配列を許容しており、「回答必須」を満たせません。
requiredだけではanswers: []を拒否できないため、最低1件を強制するminItemsが必要です。修正案
answers: type: array + minItems: 1 items: $ref: '#/components/schemas/FeedbackAnswerInput'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/v2.yaml` around lines 4247 - 4251, The OpenAPI schema for the array property named "answers" (items: $ref: '#/components/schemas/FeedbackAnswerInput') currently allows an empty array; add a minItems: 1 constraint to that "answers" array so the spec enforces at least one entry (and optionally update the description to note "最小1件必須").migrations/20260225125932_create_game_feedbacks.sql (1)
26-37:⚠️ Potential issue | 🟠 Major質問と回答先フィードバックの同一ゲーム性をDBで担保できていません。
question_idとfeedback_idの外部キーが独立しているため、別ゲームの質問を別ゲームのフィードバックに紐づける不整合データを作れてしまいます。
DB制約(または少なくとも厳密なサービス層検証)で同一ゲーム性を強制してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@migrations/20260225125932_create_game_feedbacks.sql` around lines 26 - 37, Add a game_id column to game_feedback_answers and enforce composite foreign keys so the feedback and question must belong to the same game: add `game_id` (same type as game id in game_feedbacks/feedback_questions) to `game_feedback_answers`, then replace/augment the independent FKs with composite FKs that reference the pair `(id, game_id)` on `game_feedbacks` and `feedback_questions` (e.g. CONSTRAINTs referencing `(feedback_id, game_id)` -> `game_feedbacks`(id, game_id) and `(question_id, game_id)` -> `feedback_questions`(id, game_id)); ensure `game_feedbacks` and `feedback_questions` expose a unique/indexed `(id, game_id)` key to be referenced. This guarantees the question and feedback share the same game at the DB level.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/20260225125932_create_game_feedbacks.sql`:
- Around line 16-24: Add an explicit edition_id column to the game_feedbacks
table so submitted feedback deterministically records the edition used: modify
the CREATE TABLE `game_feedbacks` to include `edition_id` varchar(36) NOT NULL
(or NULL if you need a transitional period), add an INDEX
`idx_game_feedbacks_edition_id`, and add a foreign key CONSTRAINT
`fk_game_feedbacks_edition` referencing your editions table (`editions` or
`v2_editions`) on the editions.id column with ON UPDATE RESTRICT ON DELETE
RESTRICT; ensure the app populates `edition_id` when inserting into
`game_feedbacks` so you no longer rely on `edition_game_version_relations` to
infer edition.
---
Duplicate comments:
In `@docs/openapi/v2.yaml`:
- Around line 1578-1579: The OpenAPI operation for putFeedbackQuestions is using
GameMaintainerAuth which grants too-broad permissions; update the operation's
security entry so that putFeedbackQuestions requires AdminAuth instead of
GameMaintainerAuth, and verify the AdminAuth security scheme is defined in the
document (adding it if missing) so the endpoint reflects admin-only access.
- Around line 4247-4251: The OpenAPI schema for the array property named
"answers" (items: $ref: '#/components/schemas/FeedbackAnswerInput') currently
allows an empty array; add a minItems: 1 constraint to that "answers" array so
the spec enforces at least one entry (and optionally update the description to
note "最小1件必須").
In `@migrations/20260225125932_create_game_feedbacks.sql`:
- Around line 26-37: Add a game_id column to game_feedback_answers and enforce
composite foreign keys so the feedback and question must belong to the same
game: add `game_id` (same type as game id in game_feedbacks/feedback_questions)
to `game_feedback_answers`, then replace/augment the independent FKs with
composite FKs that reference the pair `(id, game_id)` on `game_feedbacks` and
`feedback_questions` (e.g. CONSTRAINTs referencing `(feedback_id, game_id)` ->
`game_feedbacks`(id, game_id) and `(question_id, game_id)` ->
`feedback_questions`(id, game_id)); ensure `game_feedbacks` and
`feedback_questions` expose a unique/indexed `(id, game_id)` key to be
referenced. This guarantees the question and feedback share the same game at the
DB level.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd5e15da-d1c0-4169-a154-190956b045a7
📒 Files selected for processing (5)
docs/openapi/v2.yamlmigrations/20260225125932_create_game_feedbacks.sqlsrc/handler/v2/game_feedback.gosrc/handler/v2/openapi/openapi.gen.gosrc/repository/gorm2/schema/v2.go
| CREATE TABLE `game_feedbacks` ( | ||
| `id` varchar(36) NOT NULL, | ||
| `game_version_id` varchar(36) NOT NULL, | ||
| `comment` text NULL, | ||
| `created_at` datetime NOT NULL DEFAULT (current_timestamp()), | ||
| PRIMARY KEY (`id`), | ||
| INDEX `idx_game_feedbacks_game_version_id` (`game_version_id`), | ||
| CONSTRAINT `fk_game_feedbacks_game_version` FOREIGN KEY (`game_version_id`) REFERENCES `v2_game_versions` (`id`) ON UPDATE RESTRICT ON DELETE RESTRICT | ||
| ) CHARSET utf8mb4 COLLATE utf8mb4_general_ci; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# マイグレーションファイルの確認
git ls-files | grep -E "migrations.*\.sql$" | head -20Repository: traPtitech/trap-collection-server
Length of output: 357
🏁 Script executed:
# 当該マイグレーションファイルの内容確認
cat -n migrations/20260225125932_create_game_feedbacks.sqlRepository: traPtitech/trap-collection-server
Length of output: 2668
🏁 Script executed:
# 他のマイグレーションファイルで edition_id の使用状況を確認
rg "edition" migrations/ -t sqlRepository: traPtitech/trap-collection-server
Length of output: 2581
🏁 Script executed:
# PR コンテキストまたは issue `#1316` の参照がないか確認
rg "1316|edition" . --type-list | head -20Repository: traPtitech/trap-collection-server
Length of output: 466
🏁 Script executed:
# アプリケーションコード内でフィードバック関連の実装確認
find . -type f \( -name "*.go" -o -name "*.ts" -o -name "*.js" \) | head -20 | xargs grep -l "feedback\|edition" 2>/dev/null | head -10Repository: traPtitech/trap-collection-server
Length of output: 59
🏁 Script executed:
# app ディレクトリの構造確認
find app -type f -name "*feedback*" 2>/dev/null | head -20Repository: traPtitech/trap-collection-server
Length of output: 59
🏁 Script executed:
# README や issues, PR テンプレートなどで要件確認
cat README.md 2>/dev/null | head -50
ls -la | grep -i "issue\|pull\|pull_request"Repository: traPtitech/trap-collection-server
Length of output: 1855
🏁 Script executed:
# 実装コード内で game_version_id から edition_id への参照パターン確認
rg "game_version.*edition|edition.*game_version" -t go -t ts -A 3 -B 3Repository: traPtitech/trap-collection-server
Length of output: 5849
🏁 Script executed:
# ソースコード全体で feedback 関連の実装確認
find . -type f -name "*.go" | xargs grep -l "feedback\|Feedback" 2>/dev/null | head -20Repository: traPtitech/trap-collection-server
Length of output: 210
🏁 Script executed:
# service または handler ディレクトリでのフィードバック実装
find src -type d -name "*service\|*handler\|*repository" 2>/dev/nullRepository: traPtitech/trap-collection-server
Length of output: 59
🏁 Script executed:
# マイグレーションドキュメント確認
cat docs/migration.md 2>/dev/null | grep -A 20 -B 5 "feedback\|1316\|edition" || echo "Not found"Repository: traPtitech/trap-collection-server
Length of output: 1228
🏁 Script executed:
# git の最近の変更から issue 番号やコミットメッセージを確認
git log --oneline --all | grep -i "feedback\|1316" | head -20Repository: traPtitech/trap-collection-server
Length of output: 102
🏁 Script executed:
# game_feedback.go の内容確認
cat -n src/handler/v2/game_feedback.go | head -100Repository: traPtitech/trap-collection-server
Length of output: 1653
🏁 Script executed:
# schema/v2.go での GameFeedback 定義確認
rg "GameFeedback|type.*GameFeedback" src/repository/gorm2/schema/v2.go -A 15Repository: traPtitech/trap-collection-server
Length of output: 2311
🏁 Script executed:
# repository での feedback 関連実装確認
find src/repository -type f -name "*.go" | xargs grep -l "feedback\|Feedback" 2>/dev/nullRepository: traPtitech/trap-collection-server
Length of output: 110
🏁 Script executed:
# openapi.gen.go での feedback スキーマ確認
rg "Feedback" src/handler/v2/openapi/openapi.gen.go -A 5 -B 2 | head -50Repository: traPtitech/trap-collection-server
Length of output: 1333
🏁 Script executed:
# repository でのフィードバック関連クエリ実装
find src/repository -name "*.go" -type f | xargs grep -l "GameFeedback\|feedback" 2>/dev/nullRepository: traPtitech/trap-collection-server
Length of output: 110
🏁 Script executed:
# repository ファイルの内容確認
find src/repository -name "*.go" -type f | xargs grep -B 5 -A 10 "GameFeedback" 2>/dev/null | head -80Repository: traPtitech/trap-collection-server
Length of output: 4055
🏁 Script executed:
# QueryBuilder や JOIN パターンの確認
rg "edition_game_version_relations" src/ -A 3 -B 3 | head -50Repository: traPtitech/trap-collection-server
Length of output: 4486
edition_id を直接保持して、提出時の edition を確定的に記録する必要があります。
game_feedbacks に edition_id がないため、フィードバック送信時点でどの edition を使用していたかを確定的に特定できません。edition_game_version_relations を通じて game_version_id から edition_id を参照することは理論上可能ですが、同一 game_version が複数 edition に属する設計の場合、曖昧性が生じます。
issue #1316 の「回答にどの edition かを付与する」要件を満たすなら、提出時の edition を永続化する明示的な edition_id カラムを追加すべきです。現在この機能は未実装のため、スキーマ定義の段階で設計を改善してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@migrations/20260225125932_create_game_feedbacks.sql` around lines 16 - 24,
Add an explicit edition_id column to the game_feedbacks table so submitted
feedback deterministically records the edition used: modify the CREATE TABLE
`game_feedbacks` to include `edition_id` varchar(36) NOT NULL (or NULL if you
need a transitional period), add an INDEX `idx_game_feedbacks_edition_id`, and
add a foreign key CONSTRAINT `fk_game_feedbacks_edition` referencing your
editions table (`editions` or `v2_editions`) on the editions.id column with ON
UPDATE RESTRICT ON DELETE RESTRICT; ensure the app populates `edition_id` when
inserting into `game_feedbacks` so you no longer rely on
`edition_game_version_relations` to infer edition.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/db_schema/feedback_questions.md`:
- Around line 8-22: ドキュメントの feedback_questions テーブル定義が実際のマイグレーション
migrations/20260225125932_create_game_feedbacks.sql
と差分があるため、docs/db_schema/feedback_questions.md を実際のDB定義に合わせて再生成してください — 具体的には
feedback_questions の `answer_type`/`question_order` 表示幅をマイグレーションと同じく表示幅なしの
`tinyint`/`int` にし、`created_at` のデフォルトを `DEFAULT (current_timestamp())`
へ、`archived_at`/`deleted_at` の NULL 表記を `NULL` に揃え、`KEY` を `INDEX` に変更して外部キー制約
`CONSTRAINT fk_feedback_questions_game ... REFERENCES games(id) ON UPDATE
RESTRICT ON DELETE RESTRICT` を追記し、最後にテーブル定義のエンコーディングを `CHARSET utf8mb4 COLLATE
utf8mb4_general_ci` に更新してドキュメントとマイグレーションが完全に一致するよう修正してください。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 51f6d3fe-028d-42b7-8fa6-94576ac62f67
⛔ Files ignored due to path filters (7)
docs/db_schema/feedback_questions.svgis excluded by!**/*.svgdocs/db_schema/game_feedback_answers.svgis excluded by!**/*.svgdocs/db_schema/game_feedbacks.svgis excluded by!**/*.svgdocs/db_schema/games.svgis excluded by!**/*.svgdocs/db_schema/schema.svgis excluded by!**/*.svgdocs/db_schema/v2_game_versions.svgis excluded by!**/*.svgmigrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
docs/db_schema/README.mddocs/db_schema/feedback_questions.mddocs/db_schema/game_feedback_answers.mddocs/db_schema/game_feedbacks.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/db_schema/game_feedback_answers.md
- docs/db_schema/game_feedbacks.md
| ```sql | ||
| CREATE TABLE `feedback_questions` ( | ||
| `id` varchar(36) NOT NULL, | ||
| `game_id` varchar(36) NOT NULL, | ||
| `question_text` varchar(256) NOT NULL, | ||
| `answer_type` tinyint(4) NOT NULL, | ||
| `question_order` int(11) NOT NULL, | ||
| `created_at` datetime NOT NULL DEFAULT current_timestamp(), | ||
| `archived_at` datetime DEFAULT NULL, | ||
| `deleted_at` datetime DEFAULT NULL, | ||
| PRIMARY KEY (`id`), | ||
| KEY `idx_feedback_questions_game_id` (`game_id`), | ||
| CONSTRAINT `fk_feedback_questions_game` FOREIGN KEY (`game_id`) REFERENCES `games` (`id`) | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if migration file exists and read its content
if [ -f "migrations/20260225125932_create_game_feedbacks.sql" ]; then
echo "=== Actual Migration SQL ==="
cat -n migrations/20260225125932_create_game_feedbacks.sql
else
echo "Migration file not found at expected path"
# Search for alternative migration files
fd "create_game_feedbacks" --type f
fiRepository: traPtitech/trap-collection-server
Length of output: 2697
🏁 Script executed:
# Also check the documentation file
echo "=== Documentation File ==="
cat -n docs/db_schema/feedback_questions.mdRepository: traPtitech/trap-collection-server
Length of output: 2328
🏁 Script executed:
# Search for any CREATE TABLE feedback_questions in migrations directory
rg "CREATE TABLE.*feedback_questions" migrations/ -A 15Repository: traPtitech/trap-collection-server
Length of output: 1586
ドキュメントと実際のマイグレーションSQLの不一致を修正してください。
ドキュメント(lines 8-22)は実際のマイグレーションファイル(migrations/20260225125932_create_game_feedbacks.sql)と以下の点で異なっています:
- Line 13-14: ドキュメントは
tinyint(4)とint(11)を使用していますが、マイグレーションでは表示幅なしのtinyintとintです。 - Line 15: ドキュメントは
DEFAULT current_timestamp()ですが、マイグレーションはDEFAULT (current_timestamp())です。 - Line 16-17: ドキュメントは
DEFAULT NULLですが、マイグレーションはNULLです。 - Line 19-20: ドキュメントは
KEYを使用していますが、マイグレーションはINDEXです。FK制約にマイグレーションで定義されているON UPDATE RESTRICT ON DELETE RESTRICT句も欠落しています。 - Line 21: ドキュメントは
ENGINE=InnoDB DEFAULT CHARSET=utf8mb4ですが、マイグレーションはCHARSET utf8mb4 COLLATE utf8mb4_general_ciです。
実際のデータベーススキーマからドキュメントを再生成して、両者を一致させてください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/db_schema/feedback_questions.md` around lines 8 - 22, ドキュメントの
feedback_questions テーブル定義が実際のマイグレーション
migrations/20260225125932_create_game_feedbacks.sql
と差分があるため、docs/db_schema/feedback_questions.md を実際のDB定義に合わせて再生成してください — 具体的には
feedback_questions の `answer_type`/`question_order` 表示幅をマイグレーションと同じく表示幅なしの
`tinyint`/`int` にし、`created_at` のデフォルトを `DEFAULT (current_timestamp())`
へ、`archived_at`/`deleted_at` の NULL 表記を `NULL` に揃え、`KEY` を `INDEX` に変更して外部キー制約
`CONSTRAINT fk_feedback_questions_game ... REFERENCES games(id) ON UPDATE
RESTRICT ON DELETE RESTRICT` を追記し、最後にテーブル定義のエンコーディングを `CHARSET utf8mb4 COLLATE
utf8mb4_general_ci` に更新してドキュメントとマイグレーションが完全に一致するよう修正してください。
ikura-hamu
left a comment
There was a problem hiding this comment.
ありがとう、必要な機能は十分あると思います。何点か修正をお願いします。
| - gameFeedback | ||
| security: | ||
| - GameMaintainerAuth: [] | ||
| - EditionAuth: [] |
There was a problem hiding this comment.
EditionAuth だと、アクセストークンの有無しか見ないので、1つのエディションのトークンを持ってるランチャーが、他のエディションにしか含まれないゲームの質問を取得できてしまう。
現在の認証方式だとこれを防ぐことはできないので、必要性も含めてあとで考える。
| 配列の順序がそのままquestion_orderになります。 | ||
| idを指定した質問は既存の質問を更新し、idを省略した質問は新規作成されます。 | ||
| リクエストに含まれなかった既存の質問はアーカイブされます(過去の回答は引き続き取得可能)。 | ||
| enabledでフィードバック機能のon/offを切り替えられます。 |
There was a problem hiding this comment.
質問の内容と、質問を有効にするかどうかは別の概念なので、別のAPIを用意したさがあります。
| tags: | ||
| - gameFeedback | ||
| security: | ||
| - EditionAuth: [] |
There was a problem hiding this comment.
GET /games/{gameID}/feedback-questions と同様
| - answers | ||
| additionalProperties: false | ||
|
|
||
| FeedbackAnswerInput: |
There was a problem hiding this comment.
質問の種類(YesNo/5段階)によって、bodyを分けた方がいいと思います。
OpenAPI の oneOf を使うと、UNION型みたいなことができるので、それを使って表現するといいと思います。(Goの生成されるコードは微妙ですが)
| $ref: '#/components/schemas/GameFeedbackID' | ||
| gameVersionID: | ||
| $ref: '#/components/schemas/GameVersionID' | ||
| gameVersionName: |
There was a problem hiding this comment.
game version IDがあればgame version nameはAPIから得られると思うのですが、レスポンスに含めた意図はなんですか_
| - createdAt | ||
| additionalProperties: false | ||
|
|
||
| FeedbackAnswer: |
| description: | | ||
| 指定したゲームのフィードバックを取得します。 | ||
| 各フィードバックにはバージョン情報が含まれます。 | ||
| アーカイブ済みの質問に対する回答も含まれます。 |
| QuestionOrder int `gorm:"type:int;not null"` | ||
| CreatedAt time.Time `gorm:"type:datetime;not null;default:CURRENT_TIMESTAMP"` | ||
| ArchivedAt sql.NullTime `gorm:"type:DATETIME NULL;default:NULL"` | ||
| DeletedAt gorm.DeletedAt `gorm:"type:DATETIME NULL;default:NULL"` |
There was a problem hiding this comment.
質問のアーカイブはArchivedAtがあり、質問を削除することは現時点では無いので、DeletedAtはいらなそう
| return "game_feedback_configs" | ||
| } | ||
|
|
||
| type FeedbackQuestionTable struct { |
There was a problem hiding this comment.
他はGameFeedbackだけどここだけFeedbackなので、揃えた方がよさそう
| ID uuid.UUID `gorm:"type:varchar(36);not null;primaryKey"` | ||
| FeedbackID uuid.UUID `gorm:"type:varchar(36);not null;index;uniqueIndex:idx_game_feedback_answers_feedback_question"` | ||
| QuestionID uuid.UUID `gorm:"type:varchar(36);not null;index;uniqueIndex:idx_game_feedback_answers_feedback_question"` | ||
| Answer int `gorm:"type:int;not null"` |
There was a problem hiding this comment.
YesNoと5段階がここに格納されると思うんですが、5段階は1-5がそのまま入るのでいいと思うんだけど、YesNoは、1と0をconst.goで定数で定義しておくといいかもしれません。なんなら型も type GameFeedbackAnswerAnswer int (名前ひどいけど)みたいなのを作って、
const (
GameFeedbackAnswerAnswerYes GameFeedbackAnswerAnswer = 1
GameFeedbackAnswerAnswerNo GameFeedbackAnswerAnswer = 0
)みたいにしてもいい。
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
docs/openapi/v2.yaml (2)
1716-1718:⚠️ Potential issue | 🟠 Major必須回答の仕様をスキーマで表現できていません。
answersは required でも空配列を許しているので、実質的に「回答なし送信」が通ります。説明文も任意に読めるため、少なくともminItems: 1を付けて文言も必須前提に揃えたいです。修正案
description: | ランチャーからゲームに対するフィードバックを送信します。 - 質問に対する回答を含むことができます。 + 少なくとも1件の質問回答を含むフィードバックを送信します。 @@ answers: type: array + minItems: 1 items: $ref: '#/components/schemas/FeedbackAnswerInput'Also applies to: 4353-4365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/v2.yaml` around lines 1716 - 1718, The OpenAPI schema for the request that includes the answers array currently marks `answers` as required but allows an empty array, so submissions with no answers pass validation; update the schema for the `answers` property to include "minItems: 1" (and ensure the property remains in the parent schema's required list) so at least one item is required, and update the description text to reflect that answers must be present; apply the same change to the other occurrence noted (the schema around lines 4353-4365) so both schemas enforce non-empty `answers`.
1577-1578:⚠️ Potential issue | 🟠 Major質問更新の権限が広すぎます。
putFeedbackQuestionsをGameMaintainerAuthで公開すると、「ゲーム制作者が質問を変更する仕組みは今回は実装しない」という要件とズレます。少なくとも管理者権限に寄せるか、このPRでは更新 API 自体を外した方がよいです。修正案
security: - - GameMaintainerAuth: [] + - AdminAuth: []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openapi/v2.yaml` around lines 1577 - 1578, putFeedbackQuestions is currently exposed under GameMaintainerAuth which is too permissive relative to the requirement that game creators should not be able to modify questions; update the OpenAPI entry for the putFeedbackQuestions operation in docs/openapi/v2.yaml to use a stricter security scheme (e.g., AdminAuth) or remove the operation from this PR entirely so it’s not publicly documented; locate the putFeedbackQuestions path and replace the security block referencing GameMaintainerAuth with the appropriate AdminAuth scheme or delete the operation definition as decided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/openapi/v2.yaml`:
- Around line 1571-1574: The batch-replace feedback-questions endpoint must not
allow in-place mutation of question semantics: when a request item references an
existing question id, only reuse the id for ordering; if questionText or
answerType differ from the stored record, treat it as a new question (create new
id and archive the old question preserving past answers) or reject with a 400
validation error. Update the API contract and the implementation that handles
the "replace feedback questions" operation to (a) validate each incoming item
against stored questionText/answerType, (b) if differing, either respond with a
clear validation error referencing the question id or automatically create a new
question and archive the old one (ensure question_order is applied to the new
entity), and (c) document this behavior in the OpenAPI description so clients
cannot silently change semantics by reusing ids.
- Around line 4277-4282: The OpenAPI enum AnswerType declares "fiveScale" but
the codebase lacks handler, validation, and DB constraints for five-scale
answers; either remove "fiveScale" from the public spec or implement full
support: add request validation for AnswerType in the request-parsing/handler
layer (the functions handling answer submissions), extend the answer-processing
handlers (e.g., the submit/patch answer methods) to accept and process fiveScale
payloads, add DB-level enum constraint or mapping instead of a free tinyint
(update the model/schema for the answers table to enforce the allowed values for
yesNo/fiveScale), and add unit/integration tests covering fiveScale validation
and persistence; update OpenAPI if you choose to remove fiveScale. Ensure you
reference and update the AnswerType enum, the answer submission handlers, the
answer validation logic, and the DB model/schema.
In `@src/handler/v2/game_feedback.go`:
- Around line 14-51: The new GameFeedback handler methods (GetFeedbackConfig,
PatchFeedbackConfig, GetFeedbackQuestions, PutFeedbackQuestions,
PostGameFeedback, GetGameFeedbacks, GetGameVersionFeedbacks) are stubs returning
501 but are currently being exposed; either stop registering the GameFeedback
routes in the router or prevent the feedback paths from being included in the
published OpenAPI spec until real implementations are provided. Concretely, in
the code that wires handlers (look for where GameFeedback or NewGameFeedback is
registered in api.go) comment out or conditionally skip registering these
routes, or alternatively adjust the OpenAPI generation/registration
(openapi.gen.go usage) to exclude the feedback endpoints so they are not
advertised; revert this change prior to merging or add a feature flag guarding
route registration.
---
Duplicate comments:
In `@docs/openapi/v2.yaml`:
- Around line 1716-1718: The OpenAPI schema for the request that includes the
answers array currently marks `answers` as required but allows an empty array,
so submissions with no answers pass validation; update the schema for the
`answers` property to include "minItems: 1" (and ensure the property remains in
the parent schema's required list) so at least one item is required, and update
the description text to reflect that answers must be present; apply the same
change to the other occurrence noted (the schema around lines 4353-4365) so both
schemas enforce non-empty `answers`.
- Around line 1577-1578: putFeedbackQuestions is currently exposed under
GameMaintainerAuth which is too permissive relative to the requirement that game
creators should not be able to modify questions; update the OpenAPI entry for
the putFeedbackQuestions operation in docs/openapi/v2.yaml to use a stricter
security scheme (e.g., AdminAuth) or remove the operation from this PR entirely
so it’s not publicly documented; locate the putFeedbackQuestions path and
replace the security block referencing GameMaintainerAuth with the appropriate
AdminAuth scheme or delete the operation definition as decided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30c56317-0fb0-4bab-84c7-ad88a7fbce2a
📒 Files selected for processing (5)
docs/openapi/v2.yamlsrc/handler/v2/game_feedback.gosrc/handler/v2/openapi/openapi.gen.gosrc/repository/gorm2/schema/const.gosrc/repository/gorm2/schema/v2.go
🚧 Files skipped from review as they are similar to previous changes (2)
- src/repository/gorm2/schema/v2.go
- src/handler/v2/openapi/openapi.gen.go
| AnswerType: | ||
| type: string | ||
| enum: | ||
| - yesNo | ||
| - fiveScale | ||
| description: '回答形式(yesNo: Yes/No回答、fiveScale: 5段階評価)' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== migration ==="
if [ -d migrations ]; then
fd 'create_game_feedbacks\.sql' migrations -x sh -c '
echo "--- {} ---"
rg -n "yesNo|fiveScale|answer.?type|feedback_questions|game_feedback_answers" "{}" || true
'
fi
echo
echo "=== gorm schema ==="
if [ -f src/repository/gorm2/schema/v2.go ]; then
rg -n "FeedbackQuestion|GameFeedbackAnswer|yesNo|fiveScale|answer.?type" src/repository/gorm2/schema/v2.go || true
fi
echo
echo "=== db docs ==="
if [ -d docs/db_schema ]; then
rg -n "yesNo|fiveScale|回答形式|feedback_question" docs/db_schema || true
fiRepository: traPtitech/trap-collection-server
Length of output: 6920
🏁 Script executed:
# Check the actual migration file for answer_type definition details
cat -n migrations/20260225125932_create_game_feedbacks.sqlRepository: traPtitech/trap-collection-server
Length of output: 2668
🏁 Script executed:
# Check the GORM schema definition for answer_type
sed -n '287,330p' src/repository/gorm2/schema/v2.go | cat -nRepository: traPtitech/trap-collection-server
Length of output: 2295
🏁 Script executed:
# Look for any enum or validation constraints on answer_type
rg -n "answer_type|AnswerType" src/repository/gorm2/schema/v2.go -A 2 -B 2Repository: traPtitech/trap-collection-server
Length of output: 439
🏁 Script executed:
# Check handler code for answer type validation
find src/handler/v2 -type f -name "*.go" | head -20Repository: traPtitech/trap-collection-server
Length of output: 662
🏁 Script executed:
# Check the game_feedback handler for validation logic
cat -n src/handler/v2/game_feedback.go | head -200Repository: traPtitech/trap-collection-server
Length of output: 2135
🏁 Script executed:
# Also check if there are any type definitions or enums in the codebase
rg -n "enum.*AnswerType|AnswerType.*enum|const.*yesNo|const.*fiveScale" src/ -iRepository: traPtitech/trap-collection-server
Length of output: 59
🏁 Script executed:
# Check the OpenAPI spec for the AnswerType definition
sed -n '4270,4290p' docs/openapi/v2.yaml | cat -nRepository: traPtitech/trap-collection-server
Length of output: 658
🏁 Script executed:
# Check if there are any actual implementations of feedback endpoints or just stubs
rg -n "PostGameFeedback|GetFeedbackQuestions|PutFeedbackQuestions" src/handler/v2/ -A 5Repository: traPtitech/trap-collection-server
Length of output: 9159
🏁 Script executed:
# Look for any validation code for answer types elsewhere
rg -n "yesNo|fiveScale" src/ --type go -B 2 -A 2Repository: traPtitech/trap-collection-server
Length of output: 7979
🏁 Script executed:
# Check what the generated openapi types look like
find src/handler/v2/openapi -name "*.go" | head -5 && rg -n "AnswerType" src/handler/v2/openapi/ | head -20Repository: traPtitech/trap-collection-server
Length of output: 2182
API仕様で公開する fiveScale に対応する実装がない。
OpenAPI spec で AnswerType enum が yesNo と fiveScale を公開していますが、以下の状態です:
- ハンドラーはすべて NotImplemented (501) で実装されていない
- データベースはtinyint型で enum制約がなく、どの値でも受け入れる
- 回答の検証ロジックがない
ハンドラーを実装する際に、fiveScale 対応が漏れると生成クライアントは5段階回答を送信できるのに拒否される可能性があります。今のうち以下いずれかの対応が必要です:
- API spec を
yesNoのみに戻す、または - このPRで
fiveScaleのハンドラー実装と検証も併せて完成させる
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/openapi/v2.yaml` around lines 4277 - 4282, The OpenAPI enum AnswerType
declares "fiveScale" but the codebase lacks handler, validation, and DB
constraints for five-scale answers; either remove "fiveScale" from the public
spec or implement full support: add request validation for AnswerType in the
request-parsing/handler layer (the functions handling answer submissions),
extend the answer-processing handlers (e.g., the submit/patch answer methods) to
accept and process fiveScale payloads, add DB-level enum constraint or mapping
instead of a free tinyint (update the model/schema for the answers table to
enforce the allowed values for yesNo/fiveScale), and add unit/integration tests
covering fiveScale validation and persistence; update OpenAPI if you choose to
remove fiveScale. Ensure you reference and update the AnswerType enum, the
answer submission handlers, the answer validation logic, and the DB
model/schema.
There was a problem hiding this comment.
@mathsuky 承知しました。fiveScale の実装は今後のPRで対応いただく形ですね。
フォローアップとして、fiveScale ハンドラー実装・バリデーション・DB制約の追加を追跡するためのGitHub Issueを作成しましょうか?
| func (gf *GameFeedback) GetFeedbackConfig(c echo.Context, _ openapi.GameIDInPath) error { | ||
| return c.NoContent(http.StatusNotImplemented) | ||
| } | ||
|
|
||
| // フィードバック設定の更新 | ||
| // (PATCH /games/{gameID}/feedback-config) | ||
| func (gf *GameFeedback) PatchFeedbackConfig(c echo.Context, _ openapi.GameIDInPath) error { | ||
| return c.NoContent(http.StatusNotImplemented) | ||
| } | ||
|
|
||
| // フィードバック質問一覧の取得 | ||
| // (GET /games/{gameID}/feedback-questions) | ||
| func (gf *GameFeedback) GetFeedbackQuestions(c echo.Context, _ openapi.GameIDInPath) error { | ||
| return c.NoContent(http.StatusNotImplemented) | ||
| } | ||
|
|
||
| // フィードバック質問の一括設定 | ||
| // (PUT /games/{gameID}/feedback-questions) | ||
| func (gf *GameFeedback) PutFeedbackQuestions(c echo.Context, _ openapi.GameIDInPath) error { | ||
| return c.NoContent(http.StatusNotImplemented) | ||
| } | ||
|
|
||
| // ゲームフィードバックの送信 | ||
| // (POST /games/{gameID}/feedbacks) | ||
| func (gf *GameFeedback) PostGameFeedback(c echo.Context, _ openapi.GameIDInPath) error { | ||
| return c.NoContent(http.StatusNotImplemented) | ||
| } | ||
|
|
||
| // ゲームのフィードバック一覧取得 | ||
| // (GET /games/{gameID}/feedbacks) | ||
| func (gf *GameFeedback) GetGameFeedbacks(c echo.Context, _ openapi.GameIDInPath, _ openapi.GetGameFeedbacksParams) error { | ||
| return c.NoContent(http.StatusNotImplemented) | ||
| } | ||
|
|
||
| // ゲームバージョンのフィードバック一覧取得 | ||
| // (GET /games/{gameID}/versions/{gameVersionID}/feedbacks) | ||
| func (gf *GameFeedback) GetGameVersionFeedbacks(c echo.Context, _ openapi.GameIDInPath, _ openapi.GameVersionIDInPath, _ openapi.GetGameVersionFeedbacksParams) error { | ||
| return c.NoContent(http.StatusNotImplemented) |
There was a problem hiding this comment.
501スタブのままルートを公開しないでください。
OpenAPI では 200/201 系の正常応答を公開しているのに、src/handler/v2/api.go / src/handler/v2/openapi/openapi.gen.go 経由でこのハンドラがそのまま有効になると、新設したフィードバック API が常時 501 を返す状態で出ます。実装を別PRに分けるなら、このPRではルータ登録か仕様公開のどちらかを止めておいた方が安全です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/handler/v2/game_feedback.go` around lines 14 - 51, The new GameFeedback
handler methods (GetFeedbackConfig, PatchFeedbackConfig, GetFeedbackQuestions,
PutFeedbackQuestions, PostGameFeedback, GetGameFeedbacks,
GetGameVersionFeedbacks) are stubs returning 501 but are currently being
exposed; either stop registering the GameFeedback routes in the router or
prevent the feedback paths from being included in the published OpenAPI spec
until real implementations are provided. Concretely, in the code that wires
handlers (look for where GameFeedback or NewGameFeedback is registered in
api.go) comment out or conditionally skip registering these routes, or
alternatively adjust the OpenAPI generation/registration (openapi.gen.go usage)
to exclude the feedback endpoints so they are not advertised; revert this change
prior to merging or add a feature flag guarding route registration.
There was a problem hiding this comment.
@mathsuky 了解です。将来の実装予定として承知しました。
ただ、このPRがそのままmainにマージされると、フィードバック系エンドポイントが常時501を返す状態で本番公開されることになります。実装が整うまでの間、src/handler/v2/api.go での *GameFeedback の登録をコメントアウトするか、フィーチャーフラグでルーター登録をガードしておくことを検討いただけると安全です。よろしければフォローアップIssueを作成しましょうか?
🧠 Learnings used
Learnt from: CR
Repo: traPtitech/trap-collection-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T10:06:55.144Z
Learning: Applies to src/handler/v2/openapi/** : Do not directly edit generated OpenAPI code in `src/handler/v2/openapi`. Hand-write implementation code in `src/handler/v2/*` instead.
Learnt from: CR
Repo: traPtitech/trap-collection-server PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T10:06:55.144Z
Learning: Applies to docs/openapi/v2.yaml : Maintain OpenAPI v2 schema in `docs/openapi/v2.yaml` and regenerate handler code with `oapi-codegen`. Hand-write implementations in `src/handler/v2/*`, never directly edit generated code in `src/handler/v2/openapi`.
|
@ikura-hamu
これについて迷う部分もあるので次のミーティングなどで相談させていただければと思います。 |
ikura-hamu
left a comment
There was a problem hiding this comment.
ありがとう、いいとおもいます。issueを立てて実装に入っちゃってください
User description
close #1316
基本的に上記issueに書かれた仕様を元に設計しましたが,いくつかの仮定を置きました
質問をゲーム制作者から変更することはない,ということでfeedback_questionテーブルはかなり限定的にして,Yes/No形式の質問のみを扱うようなものにしましたが,これが良いものであるかは分かりません。(実際,自由記述はfeedbackそのものにつける様な形式になっています。)
もしかしたらfeedback_questionテーブルで質問の種類をenumで持つとか,もうちょっと質問の自由度を許容する様な設計にしても良いのでは?と悩んでいます。
PR Type
Enhancement
Description
フィードバック機能用の新APIエンドポイント追加
DBスキーマとマイグレーション追加
feedback_questions,game_feedbacks,game_feedback_answersGORMモデル定義を追加
ハンドラに未実装メソッドを仮置き
Diagram Walkthrough
flowchart LR subgraph API Q[getFeedbackQuestions] P[postGameFeedback] GF[getGameFeedbacks] GV[getGameVersionFeedbacks] end Q --> DBQ["feedback_questionsテーブル参照"] P --> GFT["game_feedbacksテーブル登録"] P --> GFAT["game_feedback_answersテーブル登録"] GF --> GFT GV --> GFTFile Walkthrough
3 files
GameFeedbackハンドラをAPI構造体に追加フィードバック用ハンドラメソッドを雛形追加フィードバック関連のGORMモデル定義追加7 files
フィードバックテーブルをスキーマREADMEに追記`editions`テーブルにgame_feedbacks参照追加`feedback_questions`スキーマドキュメント新規作成`game_feedback_answers`スキーマドキュメント新規作成`game_feedbacks`スキーマドキュメント新規作成`v2_game_versions`にgame_feedbacks参照を追加フィードバックAPI定義とスキーマを追加2 files
フィードバック用テーブル作成マイグレーション追加新マイグレーションのサム更新Summary by CodeRabbit
リリースノート
新機能
ドキュメント