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

fix article-forward router #292

Conversation

karta0807913
Copy link
Contributor

👏 解決掉的 issue / Resolved Issues

⛏ 變更內容 / Details of Changes

  • change parameter board to board_id (文件裡面是寫 board_id 而非 board)
  • add permission type PermissionForwardArticleToBoard and PermissionForwardArticleToEmail
  • remove permission type PermissionForwardArticle and PermissionForwardAddArticle
  • fix test file C6-1-7.sh and C6-1-8.sh in testing-script
  • remove redundant test in function usecase.ForwardArticleToBoard (我看其他的函數都沒有檢查,只有這個有,所以我就統一都刪掉了)
  • add error NoPermissionForForwardArticleError and NoPermissionForForwardArticleToEmailError

* change parameter `board` to `board_id`
* add permission type `PermissionForwardArticleToBoard` and
  `PermissionForwardArticleToEmail`
* remove permission type `PermissionForwardArticle` and
  `PermissionForwardAddArticle`
* fix test file `C6-1-7.sh` and `C6-1-8.sh` in testing-script
* remove redundant test in function `usecase.ForwardArticleToBoard`
* add error `NoPermissionForForwardArticleError` and
  `NoPermissionForForwardArticleToEmailError`
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2021

Codecov Report

Merging #292 (5cafd6f) into development (3e17e31) will increase coverage by 0.70%.
The diff coverage is 53.84%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #292      +/-   ##
===============================================
+ Coverage        44.04%   44.74%   +0.70%     
===============================================
  Files               29       29              
  Lines             1712     1770      +58     
===============================================
+ Hits               754      792      +38     
- Misses             848      868      +20     
  Partials           110      110              
Impacted Files Coverage Δ
internal/usecase/article.go 38.70% <ø> (+4.42%) ⬆️
internal/usecase/token.go 0.00% <0.00%> (ø)
internal/delivery/http/errors.go 45.61% <54.54%> (+22.20%) ⬆️
internal/delivery/http/route_forward_article.go 81.57% <63.63%> (+13.06%) ⬆️
internal/delivery/http/route.go 48.85% <100.00%> (-1.44%) ⬇️
internal/delivery/http/route_append_comment.go 71.62% <0.00%> (-7.49%) ⬇️
internal/config/config.go 77.27% <0.00%> (ø)
internal/repository/article.go 0.00% <0.00%> (ø)
internal/delivery/http/route_create_article.go 48.38% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e17e31...5cafd6f. Read the comment docs.

"board_id": toBoard,
err := delivery.usecase.CheckPermission(token, []usecase.Permission{usecase.PermissionForwardArticleToBoard}, map[string]string{
"board_id": boardID,
"to_board": toBoard,
Copy link
Member

Choose a reason for hiding this comment

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

這邊的參數名稱感覺需要文件化

Copy link
Contributor Author

Choose a reason for hiding this comment

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

這邊的話我看大家都是這麼放的

// Check permission for append comment
err = delivery.usecase.CheckPermission(token, []usecase.Permission{usecase.PermissionAppendComment}, map[string]string{
"board_id": boardID,
"article_id": filename,
"user_id": userID,
})

func (usecase *usecase) checkAppendCommentPermission(token string, userInfo map[string]string) error {
// boardID := userInfo["board_id"]
// userID := userInfo["user_id"]
// TODO: 判斷在該版是否被水桶
// TODO: 判斷該版是否允許推文
// TODO: 判斷該文章是否鎖文
return nil
}

如果需要文件化的話請問要放在哪裡呢?

Copy link
Member

Choose a reason for hiding this comment

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

checkAppendCommentPermission 的前面

Copy link
Member

@PichuChen PichuChen left a comment

Choose a reason for hiding this comment

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

有些函式的註解需要補充

@karta0807913 karta0807913 requested a review from PichuChen October 2, 2021 08:29
Copy link
Member

@PichuChen PichuChen left a comment

Choose a reason for hiding this comment

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

LGTM

@PichuChen PichuChen merged commit e42fab6 into Ptt-official-app:development Oct 13, 2021
@karta0807913 karta0807913 deleted the feature/#286-#fix-forward-article-route branch October 14, 2021 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] C6-1-7, C6-1-8: 轉發文章時出現 path not found
4 participants