feat: add comment support for problems #93
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #93 +/- ##
==========================================
- Coverage 72.51% 72.00% -0.52%
==========================================
Files 56 58 +2
Lines 5851 6190 +339
==========================================
+ Hits 4243 4457 +214
- Misses 1355 1468 +113
- Partials 253 265 +12
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
app/controller/comment.go
Outdated
| @@ -128,24 +156,24 @@ func AddReaction(c echo.Context) error { | |||
| return err | |||
| } | |||
| user, ok := c.Get("user").(models.User) | |||
| targetID,_ := strconv.Atoi(c.Param("id")) | |||
| if !ok { | |||
There was a problem hiding this comment.
这个要紧挨着158行
以及，可以直接user := c.Get("user").(models.User)。这么写会在不ok的时候panic。
加上ok是想要不panic的时候用的。
app/controller/comment.go
Outdated
| if !ok { | ||
| panic("could not convert my user into type models.User") | ||
| } | ||
|
|
||
| targetID := req.TargetID |
There was a problem hiding this comment.
这个其实是传入的comment_id，就是 /api/comment/:id/reaction里的id，我在request里传了份一样的
There was a problem hiding this comment.
@yeyn19 我的意思是，为啥不直接用req.TargetID，非得复制一个变量
There was a problem hiding this comment.
嗷嗷这个其实是之前没有bind时候的逻辑…后来没改…
app/controller/comment.go
Outdated
| if !ok { | ||
| panic("could not convert my user into type models.User") | ||
| } | ||
|
|
||
| targetID := req.TargetID |
app/controller/comment.go
Outdated
| //operator is not zero | ||
| for _, v := range maps[typeId] { | ||
| if uint(v) == uint(user.ID) { | ||
| return c.JSON(http.StatusBadRequest, response.ErrorResp("can't action twice!", nil)) |
There was a problem hiding this comment.
是否理解成，第二次reaction意思就是撤回？
现在有单独的撤回api吗？
There was a problem hiding this comment.
现在是AddReactionRequest中的IfAddReaction代表是增加还是删除，在前端有根据用户是否已经对某条评论点赞做了parse，理论上不会出现“重复点赞”或者“没点赞直接删除”的情况…
There was a problem hiding this comment.
感觉IfAddReaction的名字不是很好🤔 有啥更简短的名字吗
There was a problem hiding this comment.
先这个吧。感觉要不就得叫 AddOrDeleteAction string 之类的反而挺麻烦
There was a problem hiding this comment.
@yeyn19
是否分成两个api，POST /comment/:id/reaction新建，DELETE /comment/:id/reaction 删除？
| newReaction := models.Reaction{ | ||
| TargetType: "comment", | ||
| } | ||
| utils.PanicIfDBError(base.DB.Save(&newReaction), "could not save reaction") |
There was a problem hiding this comment.
能不能在发reaction的时候再创建reaction表里的数据？
There was a problem hiding this comment.
在save comment时好像不指定对应reaction就会报错violates foreign key constraint，不知道是不是comment.reaction有polymorphic的原因…
| if comment.ID == 0 { | ||
| return nil | ||
| } | ||
| if err := tx.Where("father_id = ?", comment.ID).Delete(&Comment{}).Error; err != nil { |
There was a problem hiding this comment.
这个贼奇怪…好像不这么写就有bug，这个特判调试的时候进入comment.id==0的情况然后卡死了…猜测是不是查father_id时，如果查不到就会进入id=0的AfterDelete函数
yeyn19
force-pushed
the
yynDevComment
branch
from
279fcff to
8b51a83
Compare
leoleoasd
force-pushed
the
yynDevComment
branch
from
8b51a83 to
d36382d
Compare
|
I have read the CLA Document and I hereby sign the CLA |
I have read the CLA Document and I hereby sign the CLA