Skip to content

Use custom type and constants to hold available order by options #2572

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 22, 2017
Merged

Use custom type and constants to hold available order by options #2572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 32 additions & 13 deletions models/repo_list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,40 @@ type SearchRepoOptions struct {
// Owner in we search search
//
// in: query
OwnerID int64 `json:"uid"`
Searcher *User `json:"-"` //ID of the person who's seeking
OrderBy string `json:"-"`
Private bool `json:"-"` // Include private repositories in results
Collaborate bool `json:"-"` // Include collaborative repositories
Starred bool `json:"-"`
Page int `json:"-"`
IsProfile bool `json:"-"`
OwnerID int64 `json:"uid"`
Searcher *User `json:"-"` //ID of the person who's seeking
OrderBy SearchOrderBy `json:"-"`
Private bool `json:"-"` // Include private repositories in results
Collaborate bool `json:"-"` // Include collaborative repositories
Starred bool `json:"-"`
Page int `json:"-"`
IsProfile bool `json:"-"`
// Limit of result
//
// maximum: setting.ExplorePagingNum
// in: query
PageSize int `json:"limit"` // Can be smaller than or equal to setting.ExplorePagingNum
}

//SearchOrderBy is used to sort the result
type SearchOrderBy string

func (s SearchOrderBy) String() string {
Copy link
Member

Choose a reason for hiding this comment

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

Actually just reviewed it again and does that function is needed? Why not to use string(x) where needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because you don't need to care what type SearchOrderBy is (try to think about it like you don't know that it's only string) and this way it implements Stringer interface. string(x) will not work without other changes if you change SearchOrderBy to int for example. Changing constants should not affect other code.

return string(s)
}

// Strings for sorting result
const (
SearchOrderByAlphabetically SearchOrderBy = "name ASC"
SearchOrderByAlphabeticallyReverse = "name DESC"
SearchOrderByLeastUpdated = "updated_unix ASC"
SearchOrderByRecentUpdated = "updated_unix DESC"
SearchOrderByOldest = "created_unix ASC"
SearchOrderByNewest = "created_unix DESC"
SearchOrderBySize = "size ASC"
SearchOrderBySizeReverse = "size DESC"
)

// SearchRepositoryByName takes keyword and part of repository name to search,
// it returns results in given range and number of total results.
func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, count int64, err error) {
Expand Down Expand Up @@ -164,7 +183,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun
}

if len(opts.OrderBy) == 0 {
opts.OrderBy = "name ASC"
opts.OrderBy = SearchOrderByAlphabetically
}

sess := x.NewSession()
Expand Down Expand Up @@ -193,7 +212,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun
if err = sess.
Where(cond).
Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
OrderBy(opts.OrderBy).
OrderBy(opts.OrderBy.String()).
Find(&repos); err != nil {
return nil, 0, fmt.Errorf("Repo: %v", err)
}
Expand All @@ -217,7 +236,7 @@ func Repositories(opts *SearchRepoOptions) (_ RepositoryList, count int64, err e

if err = x.
Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
OrderBy(opts.OrderBy).
OrderBy(opts.OrderBy.String()).
Find(&repos); err != nil {
return nil, 0, fmt.Errorf("Repo: %v", err)
}
Expand All @@ -236,7 +255,7 @@ func GetRecentUpdatedRepositories(opts *SearchRepoOptions) (repos RepositoryList
var cond = builder.NewCond()

if len(opts.OrderBy) == 0 {
opts.OrderBy = "updated_unix DESC"
opts.OrderBy = SearchOrderByRecentUpdated
}

if !opts.Private {
Expand Down Expand Up @@ -270,7 +289,7 @@ func GetRecentUpdatedRepositories(opts *SearchRepoOptions) (repos RepositoryList
if err = x.Where(cond).
Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
Limit(opts.PageSize).
OrderBy(opts.OrderBy).
OrderBy(opts.OrderBy.String()).
Find(&repos); err != nil {
return nil, 0, fmt.Errorf("Repo: %v", err)
}
Expand Down
18 changes: 9 additions & 9 deletions routers/home.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,27 +86,27 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) {
repos []*models.Repository
count int64
err error
orderBy string
orderBy models.SearchOrderBy
)
ctx.Data["SortType"] = ctx.Query("sort")

switch ctx.Query("sort") {
case "oldest":
orderBy = "created_unix ASC"
orderBy = models.SearchOrderByOldest
case "recentupdate":
orderBy = "updated_unix DESC"
orderBy = models.SearchOrderByRecentUpdated
case "leastupdate":
orderBy = "updated_unix ASC"
orderBy = models.SearchOrderByLeastUpdated
case "reversealphabetically":
orderBy = "name DESC"
orderBy = models.SearchOrderByAlphabeticallyReverse
case "alphabetically":
orderBy = "name ASC"
orderBy = models.SearchOrderByAlphabetically
case "reversesize":
orderBy = "size DESC"
orderBy = models.SearchOrderBySizeReverse
case "size":
orderBy = "size ASC"
orderBy = models.SearchOrderBySize
default:
orderBy = "created_unix DESC"
orderBy = models.SearchOrderByNewest
}

keyword := strings.Trim(ctx.Query("q"), " ")
Expand Down
20 changes: 10 additions & 10 deletions routers/user/profile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,26 +107,26 @@ func Profile(ctx *context.Context) {
var (
repos []*models.Repository
count int64
orderBy string
orderBy models.SearchOrderBy
)

ctx.Data["SortType"] = ctx.Query("sort")
switch ctx.Query("sort") {
case "newest":
orderBy = "created_unix DESC"
orderBy = models.SearchOrderByNewest
case "oldest":
orderBy = "created_unix ASC"
orderBy = models.SearchOrderByOldest
case "recentupdate":
orderBy = "updated_unix DESC"
orderBy = models.SearchOrderByRecentUpdated
case "leastupdate":
orderBy = "updated_unix ASC"
orderBy = models.SearchOrderByLeastUpdated
case "reversealphabetically":
orderBy = "name DESC"
orderBy = models.SearchOrderByAlphabeticallyReverse
case "alphabetically":
orderBy = "name ASC"
orderBy = models.SearchOrderByAlphabetically
default:
ctx.Data["SortType"] = "recentupdate"
orderBy = "updated_unix DESC"
orderBy = models.SearchOrderByNewest
}

// set default sort value if sort is empty.
Expand All @@ -149,7 +149,7 @@ func Profile(ctx *context.Context) {
case "stars":
ctx.Data["PageIsProfileStarList"] = true
if len(keyword) == 0 {
repos, err = ctxUser.GetStarredRepos(showPrivate, page, setting.UI.User.RepoPagingNum, orderBy)
repos, err = ctxUser.GetStarredRepos(showPrivate, page, setting.UI.User.RepoPagingNum, orderBy.String())
if err != nil {
ctx.Handle(500, "GetStarredRepos", err)
return
Expand Down Expand Up @@ -182,7 +182,7 @@ func Profile(ctx *context.Context) {
default:
if len(keyword) == 0 {
var total int
repos, err = models.GetUserRepositories(ctxUser.ID, showPrivate, page, setting.UI.User.RepoPagingNum, orderBy)
repos, err = models.GetUserRepositories(ctxUser.ID, showPrivate, page, setting.UI.User.RepoPagingNum, orderBy.String())
if err != nil {
ctx.Handle(500, "GetRepositories", err)
return
Expand Down