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` ef90fdbd1d/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. ef90fdbd1d/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.
2024-11-24 08:57:03 -05:00 · 2023-09-28 21:21:47 +08:00 · 2023-09-28 21:21:47 +08:00 · 3fcad582c9
commit 3fcad582c9
parent 1c039b4e1e
3 changed files with 67 additions and 51 deletions
--- 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
--- 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
--- 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