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.

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 page for empty or non-existing target #24659

Merged
merged 1 commit into from May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions models/fixtures/release.yml
Expand Up @@ -108,3 +108,31 @@
is_prerelease: false
is_tag: false
created_unix: 946684803

- id: 9
repo_id: 57
publisher_id: 2
tag_name: "non-existing-target-branch"
lower_tag_name: "non-existing-target-branch"
target: "non-existing"
title: "non-existing-target-branch"
sha1: "cef06e48f2642cd0dc9597b4bea09f4b3f74aad6"
num_commits: 5
is_draft: false
is_prerelease: false
is_tag: false
created_unix: 946684803

- id: 10
repo_id: 57
publisher_id: 2
tag_name: "empty-target-branch"
lower_tag_name: "empty-target-branch"
target: ""
title: "empty-target-branch"
sha1: "cef06e48f2642cd0dc9597b4bea09f4b3f74aad6"
num_commits: 5
is_draft: false
is_prerelease: false
is_tag: false
created_unix: 946684803
1 change: 1 addition & 0 deletions models/repo/release.go
Expand Up @@ -71,6 +71,7 @@ type Release struct {
OriginalAuthorID int64 `xorm:"index"`
LowerTagName string
Target string
TargetBehind string `xorm:"-"` // to handle non-existing or empty target
Title string
Sha1 string `xorm:"VARCHAR(40)"`
NumCommits int64
Expand Down
34 changes: 22 additions & 12 deletions routers/web/repo/release.go
Expand Up @@ -5,6 +5,7 @@
package repo

import (
"errors"
"fmt"
"net/http"
"strings"
Expand All @@ -16,6 +17,7 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown"
Expand All @@ -36,24 +38,32 @@ const (

// calReleaseNumCommitsBehind calculates given release has how many commits behind release target.
func calReleaseNumCommitsBehind(repoCtx *context.Repository, release *repo_model.Release, countCache map[string]int64) error {
// Get count if not exists
if _, ok := countCache[release.Target]; !ok {
// short-circuit for the default branch
if repoCtx.Repository.DefaultBranch == release.Target || repoCtx.GitRepo.IsBranchExist(release.Target) {
commit, err := repoCtx.GitRepo.GetBranchCommit(release.Target)
if err != nil {
target := release.Target
if target == "" {
target = repoCtx.Repository.DefaultBranch
}
// Get count if not cached
if _, ok := countCache[target]; !ok {
commit, err := repoCtx.GitRepo.GetBranchCommit(target)
if err != nil {
var errNotExist git.ErrNotExist
if target == repoCtx.Repository.DefaultBranch || !errors.As(err, &errNotExist) {
return fmt.Errorf("GetBranchCommit: %w", err)
}
countCache[release.Target], err = commit.CommitsCount()
// fallback to default branch
target = repoCtx.Repository.DefaultBranch
commit, err = repoCtx.GitRepo.GetBranchCommit(target)
if err != nil {
return fmt.Errorf("CommitsCount: %w", err)
return fmt.Errorf("GetBranchCommit(DefaultBranch): %w", err)
}
} else {
// Use NumCommits of the newest release on that target
countCache[release.Target] = release.NumCommits
}
countCache[target], err = commit.CommitsCount()
if err != nil {
return fmt.Errorf("CommitsCount: %w", err)
}
}
release.NumCommitsBehind = countCache[release.Target] - release.NumCommits
release.NumCommitsBehind = countCache[target] - release.NumCommits
release.TargetBehind = target
return nil
}

Expand Down
47 changes: 47 additions & 0 deletions routers/web/repo/release_test.go
Expand Up @@ -11,6 +11,8 @@ import (
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms"

"github.com/stretchr/testify/assert"
)

func TestNewReleasePost(t *testing.T) {
Expand Down Expand Up @@ -62,3 +64,48 @@ func TestNewReleasePost(t *testing.T) {
ctx.Repo.GitRepo.Close()
}
}

func TestNewReleasesList(t *testing.T) {
unittest.PrepareTestEnv(t)
ctx := test.MockContext(t, "user2/repo-release/releases")
test.LoadUser(t, ctx, 2)
test.LoadRepo(t, ctx, 57)
test.LoadGitRepo(t, ctx)
t.Cleanup(func() { ctx.Repo.GitRepo.Close() })

Releases(ctx)
releases := ctx.Data["Releases"].([]*repo_model.Release)
type computedFields struct {
NumCommitsBehind int64
TargetBehind string
}
expectedComputation := map[string]computedFields{
"v1.0": {
NumCommitsBehind: 3,
TargetBehind: "main",
},
"v1.1": {
NumCommitsBehind: 1,
TargetBehind: "main",
},
"v2.0": {
NumCommitsBehind: 0,
TargetBehind: "main",
},
"non-existing-target-branch": {
NumCommitsBehind: 1,
TargetBehind: "main",
},
"empty-target-branch": {
NumCommitsBehind: 1,
TargetBehind: "main",
},
}
for _, r := range releases {
actual := computedFields{
NumCommitsBehind: r.NumCommitsBehind,
TargetBehind: r.TargetBehind,
}
assert.Equal(t, expectedComputation[r.TagName], actual, "wrong computed fields for %s: %#v", r.TagName, r)
}
}
4 changes: 2 additions & 2 deletions templates/repo/release/list.tmpl
Expand Up @@ -47,7 +47,7 @@
{{end}}
|
{{end}}
<span class="ahead"><a href="{{$.RepoLink}}/compare/{{.TagName | PathEscapeSegments}}{{if .Target}}...{{.Target | PathEscapeSegments}}{{end}}">{{$.locale.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}}</a> {{$.locale.Tr "repo.release.ahead.target" $.DefaultBranch}}</span>
<span class="ahead"><a href="{{$.RepoLink}}/compare/{{.TagName | PathEscapeSegments}}...{{.TargetBehind | PathEscapeSegments}}">{{$.locale.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}}</a> {{$.locale.Tr "repo.release.ahead.target" .TargetBehind}}</span>
</p>
<div class="download">
{{if $.Permission.CanRead $.UnitTypeCode}}
Expand Down Expand Up @@ -96,7 +96,7 @@
<span class="time">{{TimeSinceUnix .CreatedUnix $.locale}}</span>
{{end}}
{{if not .IsDraft}}
| <span class="ahead"><a href="{{$.RepoLink}}/compare/{{.TagName | PathEscapeSegments}}...{{.Target | PathEscapeSegments}}">{{$.locale.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}}</a> {{$.locale.Tr "repo.release.ahead.target" .Target}}</span>
| <span class="ahead"><a href="{{$.RepoLink}}/compare/{{.TagName | PathEscapeSegments}}...{{.TargetBehind | PathEscapeSegments}}">{{$.locale.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}}</a> {{$.locale.Tr "repo.release.ahead.target" .TargetBehind}}</span>
{{end}}
</p>
<div class="markup desc">
Expand Down
10 changes: 7 additions & 3 deletions tests/integration/release_test.go
Expand Up @@ -143,10 +143,10 @@ func TestViewReleaseListNoLogin(t *testing.T) {

htmlDoc := NewHTMLParser(t, rsp.Body)
releases := htmlDoc.Find("#release-list li.ui.grid")
assert.Equal(t, 3, releases.Length())
assert.Equal(t, 5, releases.Length())

links := make([]string, 0, 3)
commitsToMain := make([]string, 0, 3)
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 {
Expand All @@ -158,11 +158,15 @@ func TestViewReleaseListNoLogin(t *testing.T) {
})

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",
Expand Down