Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

コメント周りのAPIを実装 #12

Merged
merged 8 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions model/comments.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package model

import "fmt"

type Comment struct {
GormModel
ItemID int `gorm:"type:int;not null" json:"item_id"`
Expand All @@ -10,3 +12,30 @@ type Comment struct {
func (Comment) TableName() string {
return "comments"
}

type CreateCommentPayload struct {
ItemID int `json:"item_id"`
UserID string `json:"user_id"`
Comment string `json:"comment"`
}

func CreateComment(p *CreateCommentPayload) (*Comment, error) {
if p.ItemID == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

[q] これ、ItemID=0 のものは弾くということであってる?
[q] Invalid な ItemID が来たときは弾かない?弾いたほうが良さそうに見えるけど

return nil, fmt.Errorf("ItemID is required")
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
}
if p.UserID == "" {
return nil, fmt.Errorf("UserID is required")
}
if p.Comment == "" {
return nil, fmt.Errorf("Comment is required")
}
c := &Comment{
ItemID: p.ItemID,
UserID: p.UserID,
Comment: p.Comment,
}
if err := db.Create(c).Error; err != nil {
return nil, err
}
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
return c, nil
}
63 changes: 63 additions & 0 deletions model/comments_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package model

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestCreateComment(t *testing.T) {
PrepareTestDatabase()

cases := []struct {
name string
payload *CreateCommentPayload
fail bool
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
}{
{
name: "正常系",
payload: &CreateCommentPayload{
ItemID: 1,
UserID: "user1",
Comment: "comment1",
},
fail: false,
},
{
name: "異常系: ItemIDが存在しない",
payload: &CreateCommentPayload{
UserID: "user1",
Copy link
Member

Choose a reason for hiding this comment

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

[imo] 0 を弾くのって話にも関連するけど、この場合 0 値として ItemId=0 がセットされているので、これを弾くのはちょっと気になるかも

Comment: "comment1",
},
fail: true,
},
{
name: "異常系: UserIDが存在しない",
payload: &CreateCommentPayload{
ItemID: 1,
Comment: "comment1",
},
fail: true,
},
{
name: "異常系: Commentが存在しない",
payload: &CreateCommentPayload{
ItemID: 1,
UserID: "user1",
},
fail: true,
},
}

assert := assert.New(t)
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
_, err := CreateComment(tt.payload)
if tt.fail {
assert.Error(err)
} else {
assert.NoError(err)
}
})
}
}
44 changes: 40 additions & 4 deletions router/comments.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,48 @@
package router

import (
"net/http"
"fmt"
"strconv"

"github.com/labstack/echo/v4"
"github.com/traPtitech/booQ-v3/model"
)

// PostComments POST /items/:id/comments
func PostComments(c echo.Context) error {
return echo.NewHTTPError(http.StatusNotImplemented, "Not Implemented")
type PostCommentBody struct {
Text string `json:"text"`
}

type PostCommentResponse struct {
ID int `json:"id"`
}

// PostComment POST /items/:id/comments
func PostComment(c echo.Context) error {
itemIDRaw := c.Param("id")
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
itemID, err := strconv.Atoi(itemIDRaw)
if err != nil {
return invalidRequest(c, err)
}

me := getAuthorizedUser(c)
Copy link
Member

Choose a reason for hiding this comment

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

[q] これって必ず失敗しない関数なの?(この PR とは無関係の話なので一旦無視しちゃって良いけど)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

内部で呼んでるのは echo.Context.Get です

Copy link
Member

Choose a reason for hiding this comment

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

:naruhodo: ちょっとコード読んだんだけど、AuthMiddleware を通さない状態でこの関数を呼び出すと panic するので気を付けて!外部からは一切のエンドポイントを叩けなくても ok ならこの実装でも良いっちゃよいです
(それはそれとして getAuthorizedUser で panic 起きてもアプリケーションが落ちないようにしたほうがいいかも)

Copy link
Member

@Takeno-hito Takeno-hito Dec 20, 2024

Choose a reason for hiding this comment

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

:naruhodo: ちょっとコード読んだんだけど、AuthMiddleware を通さない状態でこの関数を呼び出すと panic するので気を付けて!外部からは一切のエンドポイントを叩けなくても ok という方針ならこの実装でも良いっちゃよいです
(それはそれとして getAuthorizedUser で panic 起きてもアプリケーションが落ちないようにしたほうがいいかも)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

panic するんですね :hae_zubora:


var body PostCommentBody
if err := c.Bind(&body); err != nil {
return invalidRequest(c, err)
}
if body.Text == "" {
return invalidRequest(c, fmt.Errorf("text is empty"))
}

payload := model.CreateCommentPayload{
ItemID: itemID,
UserID: me,
Comment: body.Text,
}
comment, err := model.CreateComment(&payload)
if err != nil {
return internalServerError(c, err)
}

return c.JSON(201, PostCommentResponse{ID: comment.ID})
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
}
46 changes: 46 additions & 0 deletions router/comments_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package router

import (
"testing"

"github.com/labstack/echo/v4"
"github.com/stretchr/testify/assert"
"github.com/traPtitech/booQ-v3/model"
)

func TestPostComment(t *testing.T) {
model.PrepareTestDatabase()

e := echo.New()
SetupRouting(e, CreateUserProvider("s9"))
cp-20 marked this conversation as resolved.
Show resolved Hide resolved

cases := []struct {
name string
payload string
expected int
}{
{
name: "正常系",
payload: `{"text":"テストコメント"}`,
expected: 201,
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
},
{
name: "異常系: 空文字列",
payload: `{"text":""}`,
expected: 400,
},
{
name: "異常系: パラメータ不足",
payload: `{}`,
expected: 400,
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
assert := assert.New(t)
rec := PerformMutation(e, "POST", "/api/items/1/comments", tc.payload)
assert.Equal(tc.expected, rec.Code)
})
}
}
2 changes: 1 addition & 1 deletion router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func SetupRouting(e *echo.Echo, client *UserProvider) {
apiItems.POST("/:id/owners", PostOwners)
apiItems.PATCH("/:id/owners/:ownershipid", PatchOwners)
apiItems.DELETE("/:id/owners/:ownershipid", DeleteOwners)
apiItems.POST("/:id/comments", PostComments)
apiItems.POST("/:id/comments", PostComment)
apiItems.POST("/:id/likes", PostLikes)
apiItems.DELETE("/:id/likes", DeleteLikes)

Expand Down
16 changes: 16 additions & 0 deletions router/test_common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package router

import (
"net/http/httptest"
"strings"

"github.com/labstack/echo/v4"
)

func PerformMutation(e *echo.Echo, method, path, payload string) *httptest.ResponseRecorder {
cp-20 marked this conversation as resolved.
Show resolved Hide resolved
req := httptest.NewRequest(method, path, strings.NewReader(payload))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
e.ServeHTTP(rec, req)
return rec
}
Loading