Feature/#152 forward article #169
Merged
Conversation
This file contains 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
Codecov Report
@@ Coverage Diff @@
## development #169 +/- ##
===============================================
+ Coverage 40.20% 41.40% +1.20%
===============================================
Files 22 23 +1
Lines 1000 1041 +41
===============================================
+ Hits 402 431 +29
- Misses 539 547 +8
- Partials 59 63 +4
Continue to review full report at Codecov.
|
PichuChen
requested changes
Apr 11, 2021
internal/usecase/usecase.go
Outdated
| @@ -54,6 +54,8 @@ type Usecase interface { | |||
| // GetPopularArticles returns all popular articles | |||
| GetPopularArticles(ctx context.Context) ([]repository.PopularArticleRecord, error) | |||
| AppendComment(ctx context.Context, userID, boardID, filename, appendType, text string) (map[string]interface{}, error) | |||
| // ForwardArticle returns forwarding results | |||
| ForwardArticle(ctx context.Context, userID, boardID, filename, toEmail, toBoard string) (map[string]interface{}, error) | |||
There was a problem hiding this comment.
Email 和 Board 應該可以拆成兩個 method 比較好，一來是不太會同時兩個都填參數，而來是如果未來要新增新的 Forward 目標時，可以單純新增新的 method 這樣。
|
related to #155 |
AmberFu
reviewed
Apr 12, 2021
|
@PichuChen 已修改，目前改成用interface， @AmberFu 在實作 #155 可以參考一下，雖然好像順便做掉了QQ。 |
titaneric
force-pushed
the
feature/#152-forward
branch
from
3d6fe99
to
f01eacc
Compare
PichuChen
requested changes
Apr 13, 2021
internal/usecase/token.go
Outdated
| @@ -15,6 +15,8 @@ const ( | |||
| PermissionReadTreasureInformation Permission = "READ_TREASURE_INFORMATION" | |||
| PermissionReadFavorite Permission = "READ_FAVORITE" | |||
| PermissionAppendComment Permission = "APPEND_COMMENT" | |||
| PermissionForwardArticleOut Permission = "FORWARD_ARTICLE" | |||
| PermissionForwardArticleTo Permission = "FORWARD_ARTICLE" | |||
There was a problem hiding this comment.
這邊兩個Key值一樣但是用了不一樣的變數名稱應該不太好？應該不用分Out和TO, 直接在註解下去說明這個權限主要是判斷可不可以轉出這樣？
There was a problem hiding this comment.
Sorry，這邊有點寫錯，應該key要不一樣的，會照你意思新增PermissionAddArticle，我再試著修改。
PichuChen
reviewed
Apr 13, 2021
internal/usecase/usecase.go
Outdated
| @@ -54,6 +54,8 @@ type Usecase interface { | |||
| // GetPopularArticles returns all popular articles | |||
| GetPopularArticles(ctx context.Context) ([]repository.PopularArticleRecord, error) | |||
| AppendComment(ctx context.Context, userID, boardID, filename, appendType, text string) (map[string]interface{}, error) | |||
| // ForwardArticle returns forwarding results | |||
| ForwardArticle(ctx context.Context, userID, boardID, filename string, to Forward) (map[string]interface{}, error) | |||
There was a problem hiding this comment.
這邊應該暫時還不需要這麼複雜...
Forward 另外拉成物件來實作的這件事應該要另外開ISSUE討論
PichuChen
reviewed
Apr 13, 2021
internal/usecase/usecase.go
Outdated
| @@ -54,6 +54,8 @@ type Usecase interface { | |||
| // GetPopularArticles returns all popular articles | |||
| GetPopularArticles(ctx context.Context) ([]repository.PopularArticleRecord, error) | |||
| AppendComment(ctx context.Context, userID, boardID, filename, appendType, text string) (map[string]interface{}, error) | |||
| // ForwardArticle returns forwarding results | |||
| ForwardArticle(ctx context.Context, userID, boardID, filename string, to Forward) (map[string]interface{}, error) | |||
There was a problem hiding this comment.
Suggested change
| ForwardArticle(ctx context.Context, userID, boardID, filename string, to Forward) (map[string]interface{}, error) | |
| ForwardArticleToBoard(ctx context.Context, userID, boardID, filename string, boardName string) (map[string]interface{}, error) | |
| ForwardArticleToEmail(ctx context.Context, userID, boardID, filename string, email string) (map[string]interface{}, error) |
|
@PichuChen 已更新 |
|
LGTM 應該要討論一下轉板跟轉信是否要拆開成兩個API實作 |
在Usecase中確實是分兩個，其實本來就是分兩個，只是不小心在delivery做起來了 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
👏 解決掉的 issue / Resolved Issues
⛏ 變更內容 / Details of Changes
Route
postBoardsmethod，針對action = "forward_article" 呼叫delivery.ForwardArticleroute_forward_article.go實作ForwardArticlemethod，呼叫usecase.ForwardArticlePermissionForwardArticle權限ForwardArticlemethod至Usecase interfaceForwardArticlemethodTest
forwardArticlemock test methodroute_forward_article_test.go