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.

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

Improvements of releases list and tags list #25859

Merged
merged 8 commits into from
Sep 28, 2023
+67 −51
Merged

Improvements of releases list and tags list #25859

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d5d06b0
NumReleases include drafts if CanWrite releases
Zettat123 Jul 12, 2023
86890ad
remove releasesOrTags
Zettat123 Jul 13, 2023
59e0ddd
fix test
Zettat123 Jul 13, 2023
a1229db
Merge branch 'main' into improve-tags-and-releases
Zettat123 Sep 22, 2023
99f3ec2
Merge branch 'main' into improve-tags-and-releases
Zettat123 Sep 27, 2023
6ead82c
remove TestNewReleasesList
Zettat123 Sep 27, 2023
a8050b7
add TestCalReleaseNumCommitsBehind
Zettat123 Sep 28, 2023
24f25e9
Merge branch 'main' into improve-tags-and-releases
GiteaBot Sep 28, 2023
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
5 changes: 4 additions & 1 deletion modules/context/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,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
Expand Down
98 changes: 50 additions & 48 deletions routers/web/repo/release.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -196,18 +156,60 @@ func releasesOrTags(ctx *context.Context, isTagList bool) {
}

ctx.Data["Releases"] = releases
ctx.Data["ReleasesNum"] = len(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
Expand Down
7 changes: 7 additions & 0 deletions routers/web/repo/release_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

repo_model "code.gitea.io/gitea/models/repo"
unit_model "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/web"
Expand Down Expand Up @@ -73,6 +74,12 @@ func TestNewReleasesList(t *testing.T) {
test.LoadGitRepo(t, ctx)
t.Cleanup(func() { ctx.Repo.GitRepo.Close() })

var err error
ctx.Data["NumReleases"], err = repo_model.GetReleaseCountByRepoID(ctx, ctx.Repo.Repository.ID, repo_model.FindReleasesOptions{
IncludeDrafts: ctx.Repo.CanWrite(unit_model.TypeReleases),
})
assert.NoError(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an assertion for ctx.Data["NumReleases"]?

Copy link
Contributor

@wxiaoguang wxiaoguang Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm .... I think the test code is not ideal. Calling Releases directly makes the middlewares never executed.

If it doesn't test NumReleases logic, it's better to remove this code block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for replying so late. I agree with your opinion that the test code is not ideal. Right now this unit test cannot work without the RepoAssignment middleware. I tried moving it to integration tests, then I found there was already a TestViewReleaseListNoLogin which tests the release list.

gitea/tests/integration/release_test.go

Lines 134 to 174 in 5600504

func TestViewReleaseListNoLogin(t *testing.T) {
defer tests.PrepareTestEnv(t)()
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 57, OwnerName: "user2", LowerName: "repo-release"})
link := repo.Link() + "/releases"
req := NewRequest(t, "GET", link)
rsp := MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, rsp.Body)
releases := htmlDoc.Find("#release-list li.ui.grid")
assert.Equal(t, 5, releases.Length())
links := make([]string, 0, 5)
commitsToMain := make([]string, 0, 5)
releases.Each(func(i int, s *goquery.Selection) {
link, exist := s.Find(".release-list-title a").Attr("href")
if !exist {
return
}
links = append(links, link)
commitsToMain = append(commitsToMain, s.Find(".ahead > a").Text())
})
assert.EqualValues(t, []string{
"/user2/repo-release/releases/tag/empty-target-branch",
"/user2/repo-release/releases/tag/non-existing-target-branch",
"/user2/repo-release/releases/tag/v2.0",
"/user2/repo-release/releases/tag/v1.1",
"/user2/repo-release/releases/tag/v1.0",
}, links)
assert.EqualValues(t, []string{
"1 commits", // like v1.1
"1 commits", // like v1.1
"0 commits",
"1 commits", // should be 3 commits ahead and 2 commits behind, but not implemented yet
"3 commits",
}, commitsToMain)
}

Maybe we can remove TestNewReleasesList from unit tests?


Releases(ctx)
releases := ctx.Data["Releases"].([]*repo_model.Release)
type computedFields struct {
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/release_tag_header.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<div class="gt-df">
<div class="gt-f1 gt-df gt-ac">
<h2 class="ui compact small menu header small-menu-items">
<a class="{{if .PageIsReleaseList}}active {{end}}item" href="{{.RepoLink}}/releases">{{.locale.PrettyNumber .ReleasesNum}} {{.locale.TrN .ReleasesNum "repo.release" "repo.releases"}}</a>
<a class="{{if .PageIsReleaseList}}active {{end}}item" href="{{.RepoLink}}/releases">{{.locale.PrettyNumber .NumReleases}} {{.locale.TrN .NumReleases "repo.release" "repo.releases"}}</a>
{{if $canReadCode}}
<a class="{{if .PageIsTagList}}active {{end}}item" href="{{.RepoLink}}/tags">{{.locale.PrettyNumber .NumTags}} {{.locale.TrN .NumTags "repo.tag" "repo.tags"}}</a>
{{end}}
Expand Down