fix/QuestionGroupsで存在しないcampIDだったら404を返す#363
Merged
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughQuestionGroup の取得と作成で、存在しない campID を ChangesQuestionGroup の 404 対応
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@repository/gormrepository/question_group_test.go`:
- Around line 219-229: The “Non-existent Camp” subtest in TestGetQuestionGroup
is calling the wrong repository method, so it never exercises the newly added
ErrCampNotFound branch. Update the subtest to invoke GetQuestionGroups on the
repository instance r instead of GetRoomGroups, and keep the existing assertions
against repository.ErrCampNotFound and a nil result so the question_group.go
error path is actually covered.
In `@router/question_groups_test.go`:
- Around line 141-156: The Camp Not Found subtest in TestGetQuestionGroups is
exercising the room-groups path instead of the question-groups path, so it never
verifies the new 404 mapping in question_groups.go. Update this subtest to use
MockQuestionGroupRepository.GetQuestionGroups and send the request to the
question-groups endpoint (matching the GetQuestionGroups handler) so the
NotFound response is tested against the correct API.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c64f0ed-2628-4019-9ef0-447120f3c939
📒 Files selected for processing (5)
openapi.yamlrepository/gormrepository/question_group.gorepository/gormrepository/question_group_test.gorouter/question_groups.gorouter/question_groups_test.go
akimon658
requested changes
Jun 30, 2026
akimon658
left a comment
akimon658
left a comment
Member
There was a problem hiding this comment.
良さそうです！
CodeRabbitに指摘されている箇所（取得は修正されているが作成が修正されていない、テスト対象がRoomGroupsになっている）だけ対応をお願いします 🙏
Member
|
作成のケースにどう対応するか明示していませんでしたが、たぶんこれもRoomGroupと同様にできそう rucQ/repository/gormrepository/room_group.go Lines 48 to 53 in c53a359 QuestionGroupとはちょっと |
akimon658
requested changes
Jun 30, 2026
akimon658
left a comment
akimon658
left a comment
Member
There was a problem hiding this comment.
もう少しだけ！
- OpenAPIの
/api/admin/camps/{campId}/question-groupsにも404を追加してほしいです router/question_groups.goのAdminPostQuestionGroupsでも、repository.ErrCampNotFoundをチェックしてhttp.StatusNotFoundを返すようにしてほしいです- repository、routerそれぞれに、QuestionGroupの作成についてのテストも追加してほしいです
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@openapi.yaml`:
- Around line 492-493: The response mapping is replacing the existing 400 entry
instead of adding the missing 404 entry, which leaves adminPostQuestionGroup out
of sync with the implementation. Update the OpenAPI response definitions for
this operation so the existing 400 response remains unchanged and add a separate
404 entry that references NotFound. Locate the responses block for
adminPostQuestionGroup and preserve both status codes side by side.
In `@repository/gormrepository/question_group_test.go`:
- Around line 176-188: The current NotFound test only rechecks GetQuestionGroup
and never exercises the missing-camp path, so update this case to target
CreateQuestionGroup instead. Use setup(t) and the repository methods around
QuestionGroup creation to attempt creating a group with a non-existent CampID,
then assert repository.ErrCampNotFound and a nil result. Keep the existing
NotFound-style assertions, but point them at CreateQuestionGroup rather than
GetQuestionGroup so the test covers the intended failure branch.
In `@router/question_groups.go`:
- Around line 45-47: The current ErrCampNotFound check in question group
creation only handles the GetOrCreateUser path, so a missing camp during
tx.CreateQuestionGroup still falls through as a 500. Update the Transaction flow
in the question group handler to inspect the returned error from the full
transaction, and apply the same errors.Is(err, repository.ErrCampNotFound) ->
http.StatusNotFound mapping there as well. Use the existing
repository.ErrCampNotFound handling around GetOrCreateUser and extend it to the
transaction result so missing camp IDs are consistently converted to a 404.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9761dc1b-07e3-4fa6-bbfc-699c4a35f610
📒 Files selected for processing (3)
openapi.yamlrepository/gormrepository/question_group_test.gorouter/question_groups.go
akimon658
requested changes
Jun 30, 2026
Contributor
Author
|
これなんで通らないんでしょうか....... |
akimon658
requested changes
Jul 1, 2026
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@router/question_groups_test.go`:
- Around line 400-405: The GetOrCreateUser mock in question_groups_test.go is
expecting a randomly generated userID, but the request being exercised sends
X-Forwarded-User with the value username. Update the expectation in the relevant
test case to match the forwarded-user header value that the router passes into
GetOrCreateUser, and keep the mocked UserRepository call aligned with the actual
request header used in the test.
- Line 406: The test file has a gofmt violation caused by trailing
whitespace/blank line formatting. Re-run gofmt on the affected test content in
router/question_groups_test.go (around the question_groups test block) and
remove any trailing spaces or stray empty lines so the file matches standard Go
formatting.
- Around line 407-409: `CreateQuestionGroup`
のモック戻り値数が実装と一致していません。`MockQuestionGroupRepository.EXPECT().CreateQuestionGroup(gomock.Any())`
は error 単一戻り値なので、ここでは `Return(nil, repository.ErrCampNotFound)`
のような2値返却をやめ、`CreateQuestionGroup` のシグネチャに合わせて `repository.ErrCampNotFound`
だけを返す形に修正してください。`CreateQuestionGroup` と `MockQuestionGroupRepository`
を基準に該当テストを見直し、他の同種モックも戻り値数が一致しているか確認してください。
- Around line 395-397: The request body type in the AdminPostQuestionGroup test
is incorrect: it uses api.AdminPostRoomGroupJSONRequestBody instead of the
question-group payload. Update the req initialization in question_groups_test.go
to use api.AdminPostQuestionGroupJSONRequestBody, and include the fields
expected by AdminPostQuestionGroup, especially Due and Questions, so the test
matches the endpoint being exercised.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1fe31982-4a5a-4d53-acf3-ece7366688a4
📒 Files selected for processing (2)
repository/gormrepository/question_group_test.gorouter/question_groups_test.go
akimon658
approved these changes
Jul 2, 2026
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
2 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.
close #140
QuestionGroupsで返り値が無かった場合に、campIDの存在確認をするようにした
RouterのQuestionGroupにNot Foundを追加した
OpenAPIに404 Not Foundを追加した
Summary by CodeRabbit
GET /api/camps/{campId}/question-groups、管理用POST /api/admin/camps/{campId}/question-groups）で、対象キャンプが存在しない場合は404 Not Found（"Camp not found"）を返すようになりました。結果が0件の場合は従来どおり空で返します。404を追加し、エラー応答を明確化しました。404、メッセージ（"Camp not found"）、戻り値（nil）を検証するケースを追加しました。