-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 release display and correct paging #2080
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
[] # empty |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -233,40 +233,44 @@ func GetReleaseByID(id int64) (*Release, error) { | |
return rel, nil | ||
} | ||
|
||
// GetReleasesByRepoID returns a list of releases of repository. | ||
func GetReleasesByRepoID(repoID int64, page, pageSize int) (rels []*Release, err error) { | ||
if page <= 0 { | ||
page = 1 | ||
} | ||
err = x. | ||
Desc("created_unix"). | ||
Limit(pageSize, (page-1)*pageSize). | ||
Find(&rels, Release{RepoID: repoID}) | ||
return rels, err | ||
// FindReleasesOptions describes the conditions to Find releases | ||
type FindReleasesOptions struct { | ||
IncludeDrafts bool | ||
TagNames []string | ||
} | ||
|
||
// GetReleaseCountByRepoID returns the count of releases of repository | ||
func GetReleaseCountByRepoID(repoID int64, includeDrafts bool) (int64, error) { | ||
func (opts *FindReleasesOptions) toConds(repoID int64) builder.Cond { | ||
var cond = builder.NewCond() | ||
cond = cond.And(builder.Eq{"repo_id": repoID}) | ||
|
||
if includeDrafts { | ||
return x.Where(cond).Count(&Release{}) | ||
if !opts.IncludeDrafts { | ||
cond = cond.And(builder.Eq{"is_draft": false}) | ||
} | ||
|
||
cond = cond.And(builder.Eq{"is_draft": false}) | ||
return x.Where(cond).Count(&Release{}) | ||
if len(opts.TagNames) > 0 { | ||
cond = cond.And(builder.In("tag_name", opts.TagNames)) | ||
} | ||
return cond | ||
} | ||
|
||
// GetReleasesByRepoIDAndNames returns a list of releases of repository according repoID and tagNames. | ||
func GetReleasesByRepoIDAndNames(repoID int64, tagNames []string) (rels []*Release, err error) { | ||
// GetReleasesByRepoID returns a list of releases of repository. | ||
func GetReleasesByRepoID(repoID int64, opts FindReleasesOptions, page, pageSize int) (rels []*Release, err error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above and page and pageSize is also could be fields of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it's not really related to conditions and toConds can't use them anyway |
||
if page <= 0 { | ||
page = 1 | ||
} | ||
|
||
err = x. | ||
Desc("created_unix"). | ||
In("tag_name", tagNames). | ||
Find(&rels, Release{RepoID: repoID}) | ||
Desc("created_unix", "id"). | ||
Limit(pageSize, (page-1)*pageSize). | ||
Where(opts.toConds(repoID)). | ||
Find(&rels) | ||
return rels, err | ||
} | ||
|
||
// GetReleaseCountByRepoID returns the count of releases of repository | ||
func GetReleaseCountByRepoID(repoID int64, opts FindReleasesOptions) (int64, error) { | ||
return x.Where(opts.toConds(repoID)).Count(&Release{}) | ||
} | ||
|
||
type releaseMetaSearch struct { | ||
ID []int64 | ||
Rel []*Release | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not let
repoID
as a field of FindReleasesOptions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first I did that but than it looked strange as func name is
GetReleasesByRepoID
. And alsorepoID
is mandatory conditionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
FindReleases
andCountReleases
maybe a new name?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But repoID is still allways required field and that will not easy be understandable by function signature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems right.