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

Implement #53 Get popular board list #54

Closed
wants to merge 45 commits into from
Closed

Implement #53 Get popular board list #54

wants to merge 45 commits into from

Conversation

nickyanggg
Copy link
Collaborator

📝 相關的 issue / Related Issues

⛏ 變更內容 / Details of Changes

  • 實作 delivery/http/route_boards.go 中的 getPopularBoardList
  • 實作 usecase/board.go 中的 GetPopularBoards
  • TODO: 依 BoardRecord 中的 number_of_user 遞減排序

ars0915 and others added 27 commits February 3, 2021 17:27
Co-authored-by: wagaru <wagaru@hotmail.com>
Co-authored-by: wagaru <wagaru@hotmail.com>
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

@PHChenGit
Copy link
Contributor

LGTM

@@ -86,6 +86,10 @@ func (usecase *MockUsecase) GetBoards(ctx context.Context, userID string) []bbs.
panic("Not implemented")
}

func (usecase *MockUsecase) GetPopularBoards(ctx context.Context) []bbs.BoardRecord {
panic("Not implemented")
Copy link
Member

Choose a reason for hiding this comment

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

這邊能不能不要用 panic, 而是return error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

好,待會改

@@ -24,6 +24,8 @@ type Usecase interface {
GetBoardByID(ctx context.Context, boardID string) (bbs.BoardRecord, error)
// GetBoards returns all board records
GetBoards(ctx context.Context, userID string) []bbs.BoardRecord
// GetPopularBoards returns top 100 popular board records
GetPopularBoards(ctx context.Context) []bbs.BoardRecord
Copy link
Member

Choose a reason for hiding this comment

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

回傳的部分建議增加 error

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

好~

@nickyanggg
Copy link
Collaborator Author

已修改完畢,那因為目前 GetPopularBoards 太過簡單,所以目前 error 值都回傳 nil,之後如果排序的部分可以實作或是 usecase.repo.GetBoards 也有回傳 error 值的話再更改。

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

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.

None yet

8 participants