From 3fcad582c9b9bfe66f4a346652f82b1aaf18430d Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Thu, 28 Sep 2023 21:21:47 +0800 Subject: [PATCH] Improvements of releases list and tags list (#25859) Follow #23465 and #25624 This PR introduces the following improvements: - We do not need to call `GetTags` to get tags because tags have been loaded by `RepoAssignment` https://github.com/go-gitea/gitea/blob/ef90fdbd1d7e1f62ed5bf18757e00e97817eb602/modules/context/repo.go#L663-L668 - Similarly, the number of tags and releases also have been loaded by `RepoAssignment`, so the related code has been removed from the handlers. The query condition of `GetReleaseCountByRepoID` in `RepoAssignment` has been changed to include draft releases. https://github.com/go-gitea/gitea/blob/ef90fdbd1d7e1f62ed5bf18757e00e97817eb602/modules/context/repo.go#L538-L551 - `releasesOrTags` function has been removed. The code for rendering releases list and tags list moved to `Releases` and `TagList` respectively. --- modules/context/repo.go | 5 +- routers/web/repo/release.go | 97 ++++++++++++++++---------------- routers/web/repo/release_test.go | 16 +++++- 3 files changed, 67 insertions(+), 51 deletions(-) diff --git a/modules/context/repo.go b/modules/context/repo.go index 44ae624568..395ccf4b10 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -545,7 +545,10 @@ func RepoAssignment(ctx *Context) context.CancelFunc { ctx.ServerError("GetReleaseCountByRepoID", err) return nil } - ctx.Data["NumReleases"], err = repo_model.GetReleaseCountByRepoID(ctx, ctx.Repo.Repository.ID, repo_model.FindReleasesOptions{}) + ctx.Data["NumReleases"], err = repo_model.GetReleaseCountByRepoID(ctx, ctx.Repo.Repository.ID, repo_model.FindReleasesOptions{ + // only show draft releases for users who can write, read-only users shouldn't see draft releases. + IncludeDrafts: ctx.Repo.CanWrite(unit_model.TypeReleases), + }) if err != nil { ctx.ServerError("GetReleaseCountByRepoID", err) return nil diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 91ade32ccc..61f1973125 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -71,18 +71,6 @@ func calReleaseNumCommitsBehind(repoCtx *context.Repository, release *repo_model func Releases(ctx *context.Context) { ctx.Data["PageIsReleaseList"] = true ctx.Data["Title"] = ctx.Tr("repo.release.releases") - releasesOrTags(ctx, false) -} - -// TagsList render tags list page -func TagsList(ctx *context.Context) { - ctx.Data["PageIsTagList"] = true - ctx.Data["Title"] = ctx.Tr("repo.release.tags") - releasesOrTags(ctx, true) -} - -func releasesOrTags(ctx *context.Context, isTagList bool) { - ctx.Data["DefaultBranch"] = ctx.Repo.Repository.DefaultBranch ctx.Data["IsViewBranch"] = false ctx.Data["IsViewTag"] = true // Disable the showCreateNewBranch form in the dropdown on this page. @@ -100,35 +88,13 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { listOptions.PageSize = setting.API.MaxResponseItems } - // TODO(20073) tags are used for compare feature which needs all tags - // filtering is done on the client-side atm - tagListStart, tagListEnd := 0, 0 - if isTagList { - tagListStart, tagListEnd = listOptions.GetStartEnd() - } - - tags, err := ctx.Repo.GitRepo.GetTags(tagListStart, tagListEnd) - if err != nil { - ctx.ServerError("GetTags", err) - return - } - ctx.Data["Tags"] = tags - writeAccess := ctx.Repo.CanWrite(unit.TypeReleases) ctx.Data["CanCreateRelease"] = writeAccess && !ctx.Repo.Repository.IsArchived opts := repo_model.FindReleasesOptions{ ListOptions: listOptions, - } - if isTagList { - // for the tags list page, show all releases with real tags (having real commit-id), - // the drafts should also be included because a real tag might be used as a draft. - opts.IncludeDrafts = true - opts.IncludeTags = true - opts.HasSha1 = util.OptionalBoolTrue - } else { // only show draft releases for users who can write, read-only users shouldn't see draft releases. - opts.IncludeDrafts = writeAccess + IncludeDrafts: writeAccess, } releases, err := repo_model.GetReleasesByRepoID(ctx, ctx.Repo.Repository.ID, opts) @@ -137,12 +103,6 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { return } - count, err := repo_model.GetReleaseCountByRepoID(ctx, ctx.Repo.Repository.ID, opts) - if err != nil { - ctx.ServerError("GetReleaseCountByRepoID", err) - return - } - for _, release := range releases { release.Repo = ctx.Repo.Repository } @@ -197,16 +157,59 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { ctx.Data["Releases"] = releases - pager := context.NewPagination(int(count), opts.PageSize, opts.Page, 5) + numReleases := ctx.Data["NumReleases"].(int64) + pager := context.NewPagination(int(numReleases), opts.PageSize, opts.Page, 5) pager.SetDefaultParams(ctx) ctx.Data["Page"] = pager - if isTagList { - ctx.Data["PageIsViewCode"] = !ctx.Repo.Repository.UnitEnabled(ctx, unit.TypeReleases) - ctx.HTML(http.StatusOK, tplTagsList) - } else { - ctx.HTML(http.StatusOK, tplReleasesList) + ctx.HTML(http.StatusOK, tplReleasesList) +} + +// TagsList render tags list page +func TagsList(ctx *context.Context) { + ctx.Data["PageIsTagList"] = true + ctx.Data["Title"] = ctx.Tr("repo.release.tags") + ctx.Data["IsViewBranch"] = false + ctx.Data["IsViewTag"] = true + // Disable the showCreateNewBranch form in the dropdown on this page. + ctx.Data["CanCreateBranch"] = false + ctx.Data["HideBranchesInDropdown"] = true + + listOptions := db.ListOptions{ + Page: ctx.FormInt("page"), + PageSize: ctx.FormInt("limit"), } + if listOptions.PageSize == 0 { + listOptions.PageSize = setting.Repository.Release.DefaultPagingNum + } + if listOptions.PageSize > setting.API.MaxResponseItems { + listOptions.PageSize = setting.API.MaxResponseItems + } + + opts := repo_model.FindReleasesOptions{ + ListOptions: listOptions, + // for the tags list page, show all releases with real tags (having real commit-id), + // the drafts should also be included because a real tag might be used as a draft. + IncludeDrafts: true, + IncludeTags: true, + HasSha1: util.OptionalBoolTrue, + } + + releases, err := repo_model.GetReleasesByRepoID(ctx, ctx.Repo.Repository.ID, opts) + if err != nil { + ctx.ServerError("GetReleasesByRepoID", err) + return + } + + ctx.Data["Releases"] = releases + + numTags := ctx.Data["NumTags"].(int64) + pager := context.NewPagination(int(numTags), opts.PageSize, opts.Page, 5) + pager.SetDefaultParams(ctx) + ctx.Data["Page"] = pager + + ctx.Data["PageIsViewCode"] = !ctx.Repo.Repository.UnitEnabled(ctx, unit.TypeReleases) + ctx.HTML(http.StatusOK, tplTagsList) } // ReleasesFeedRSS get feeds for releases in RSS format diff --git a/routers/web/repo/release_test.go b/routers/web/repo/release_test.go index 54118fb6b3..a5a923d464 100644 --- a/routers/web/repo/release_test.go +++ b/routers/web/repo/release_test.go @@ -7,6 +7,7 @@ import ( "testing" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/contexttest" "code.gitea.io/gitea/modules/web" @@ -65,7 +66,7 @@ func TestNewReleasePost(t *testing.T) { } } -func TestNewReleasesList(t *testing.T) { +func TestCalReleaseNumCommitsBehind(t *testing.T) { unittest.PrepareTestEnv(t) ctx, _ := contexttest.MockContext(t, "user2/repo-release/releases") contexttest.LoadUser(t, ctx, 2) @@ -73,8 +74,17 @@ func TestNewReleasesList(t *testing.T) { contexttest.LoadGitRepo(t, ctx) t.Cleanup(func() { ctx.Repo.GitRepo.Close() }) - Releases(ctx) - releases := ctx.Data["Releases"].([]*repo_model.Release) + releases, err := repo_model.GetReleasesByRepoID(ctx, ctx.Repo.Repository.ID, repo_model.FindReleasesOptions{ + IncludeDrafts: ctx.Repo.CanWrite(unit.TypeReleases), + }) + assert.NoError(t, err) + + countCache := make(map[string]int64) + for _, release := range releases { + err := calReleaseNumCommitsBehind(ctx.Repo, release, countCache) + assert.NoError(t, err) + } + type computedFields struct { NumCommitsBehind int64 TargetBehind string