You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The convertGameVisibility function could potentially return an empty string for invalid visibility values. Ensure this is handled correctly in all cases.
Error handling for convertGameVisibility in PatchGameRole and DeleteGameRole should be reviewed to ensure proper logging and user feedback.
varvisibility openapi.GameVisibilityvisibility, err=convertGameVisibility(newGameInfo.Game.GetVisibility())
iferr!=nil {
log.Printf("error: failed to convert game visibility: %v\n", err)
returnecho.NewHTTPError(http.StatusInternalServerError, "failed to convert game visibility")
if errors.Is(err, service.ErrNoGame) {
- return echo.NewHTTPError(http.StatusBadRequest, "no game")+ return echo.NewHTTPError(http.StatusNotFound, "no game")
}
Suggestion importance[1-10]: 8
Why: Changing the status code for ErrNoGame from 400 to 404 ensures consistency with standard HTTP status codes, improving the API's clarity and correctness. This is a significant improvement for API design.
-return "", fmt.Errorf("invalid game visibility: %v", value)+return "", fmt.Errorf("invalid game visibility: %v (type: %T)", value, value)
Suggestion importance[1-10]: 7
Why: Adding type information to the error message in convertGameVisibility enhances debugging by providing more context about the invalid value. This is a minor but useful improvement for maintainability and debugging.
-if req.Type != nil {- var roleType values.GameManagementRole- switch *req.Type {- case "owner":- roleType = values.GameManagementRoleAdministrator- case "maintainer":- roleType = values.GameManagementRoleCollaborator- default:- return echo.NewHTTPError(http.StatusBadRequest, "role type is invalid")- }- ...+if req.Type == nil {+ return nil
}
+var roleType values.GameManagementRole+switch *req.Type {+case "owner":+ roleType = values.GameManagementRoleAdministrator+case "maintainer":+ roleType = values.GameManagementRoleCollaborator+default:+ return echo.NewHTTPError(http.StatusBadRequest, "role type is invalid")+}+...
Suggestion importance[1-10]: 6
Why: Introducing an early return for nil checks improves code readability and reduces nesting. However, the current implementation is already functional and clear, so the impact is moderate.
-GetGameErr: errors.New("error"),+GetGameErr: errors.New("invalid visibility value encountered"),
Suggestion importance[1-10]: 5
Why: Making the error message in the test case more specific aids debugging by clearly indicating the issue. While helpful, this change has a limited impact as it only affects test output.
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
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.
User description
fix #1086
一部バグ修正含む
PR Type
Tests, Bug fix
Description
PatchGameRoleにおけるバグ修正とテスト追加DeleteGameRoleにおけるバグ修正とテスト追加新しいユーティリティ関数
convertGameVisibilityの追加PatchGameRoleとDeleteGameRoleのユニットテストを大幅に追加Changes walkthrough 📝
game.go
ゲームのvisibility変換関数を追加src/handler/v2/game.go
convertGameVisibilityを追加game_role.go
PatchGameRoleとDeleteGameRoleのバグ修正と改善src/handler/v2/game_role.go
PatchGameRoleでvisibilityとジャンルを返却する処理を追加DeleteGameRoleでvisibilityとジャンルを返却する処理を追加game_role_test.go
PatchGameRoleとDeleteGameRoleのユニットテスト追加src/handler/v2/game_role_test.go
PatchGameRoleのユニットテストを追加DeleteGameRoleのユニットテストを追加