fix some ui bug about draft release (#15137)

* fix some ui bug about draft release

- should not show draft release in tag list because
  it will't create real tag
- still show draft release without tag and commit message
  for draft release instead of 404 error
- remove tag load for attachement links because it's useless

Signed-off-by: a1012112796 <1012112796@qq.com>

* add test code

* fix test

That's because has added a new release in relaese test database.

* fix dropdown link for draft release

integrations/release_test.go

@@ -10,9 +10,11 @@ import (
 	"testing"
 	"time"
 
+	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/test"
 
+	"github.com/PuerkitoBio/goquery"
 	"github.com/stretchr/testify/assert"
 	"github.com/unknwon/i18n"
 )
@@ -83,7 +85,7 @@ func TestCreateRelease(t *testing.T) {
 	session := loginUser(t, "user2")
 	createNewRelease(t, session, "/user2/repo1", "v0.0.1", "v0.0.1", false, false)
 
-	checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.1", i18n.Tr("en", "repo.release.stable"), 2)
+	checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.1", i18n.Tr("en", "repo.release.stable"), 3)
 }
 
 func TestCreateReleasePreRelease(t *testing.T) {
@@ -92,7 +94,7 @@ func TestCreateReleasePreRelease(t *testing.T) {
 	session := loginUser(t, "user2")
 	createNewRelease(t, session, "/user2/repo1", "v0.0.1", "v0.0.1", true, false)
 
-	checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.1", i18n.Tr("en", "repo.release.prerelease"), 2)
+	checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.1", i18n.Tr("en", "repo.release.prerelease"), 3)
 }
 
 func TestCreateReleaseDraft(t *testing.T) {
@@ -101,7 +103,7 @@ func TestCreateReleaseDraft(t *testing.T) {
 	session := loginUser(t, "user2")
 	createNewRelease(t, session, "/user2/repo1", "v0.0.1", "v0.0.1", false, true)
 
-	checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.1", i18n.Tr("en", "repo.release.draft"), 2)
+	checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.1", i18n.Tr("en", "repo.release.draft"), 3)
 }
 
 func TestCreateReleasePaging(t *testing.T) {
@@ -127,3 +129,80 @@ func TestCreateReleasePaging(t *testing.T) {
 	session2 := loginUser(t, "user4")
 	checkLatestReleaseAndCount(t, session2, "/user2/repo1", "v0.0.11", i18n.Tr("en", "repo.release.stable"), 10)
 }
+
+func TestViewReleaseListNoLogin(t *testing.T) {
+	defer prepareTestEnv(t)()
+
+	repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
+
+	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, 1, releases.Length())
+
+	links := 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)
+	})
+
+	assert.EqualValues(t, []string{"/user2/repo1/releases/tag/v1.1"}, links)
+}
+
+func TestViewReleaseListLogin(t *testing.T) {
+	defer prepareTestEnv(t)()
+
+	repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
+
+	link := repo.Link() + "/releases"
+
+	session := loginUser(t, "user1")
+	req := NewRequest(t, "GET", link)
+	rsp := session.MakeRequest(t, req, http.StatusOK)
+
+	htmlDoc := NewHTMLParser(t, rsp.Body)
+	releases := htmlDoc.Find("#release-list li.ui.grid")
+	assert.Equal(t, 2, releases.Length())
+
+	links := 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)
+	})
+
+	assert.EqualValues(t, []string{"/user2/repo1/releases/tag/draft-release",
+		"/user2/repo1/releases/tag/v1.1"}, links)
+}
+
+func TestViewTagsList(t *testing.T) {
+	defer prepareTestEnv(t)()
+
+	repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
+
+	link := repo.Link() + "/tags"
+
+	session := loginUser(t, "user1")
+	req := NewRequest(t, "GET", link)
+	rsp := session.MakeRequest(t, req, http.StatusOK)
+
+	htmlDoc := NewHTMLParser(t, rsp.Body)
+	tags := htmlDoc.Find(".tag-list tr")
+	assert.Equal(t, 2, tags.Length())
+
+	tagNames := make([]string, 0, 5)
+	tags.Each(func(i int, s *goquery.Selection) {
+		tagNames = append(tagNames, s.Find(".tag a.df.ac").Text())
+	})
+
+	assert.EqualValues(t, []string{"delete-tag", "v1.1"}, tagNames)
+}

models/fixtures/release.yml

@@ -43,3 +43,15 @@
   is_tag: true
   created_unix: 946684800
 
+-
+  id: 4
+  repo_id: 1
+  publisher_id: 2
+  tag_name: "draft-release"
+  lower_tag_name: "draft-release"
+  target: "master"
+  title: "draft-release"
+  is_draft: true
+  is_prerelease: false
+  is_tag: false
+  created_unix: 1619524806

modules/context/repo.go

@@ -724,7 +724,7 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
 
 // RepoRefByType handles repository reference name for a specific type
 // of repository reference
-func RepoRefByType(refType RepoRefType) func(*Context) context.CancelFunc {
+func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context) context.CancelFunc {
 	return func(ctx *Context) (cancel context.CancelFunc) {
 		// Empty repository does not have reference information.
 		if ctx.Repo.Repository.IsEmpty {
@@ -813,6 +813,9 @@ func RepoRefByType(refType RepoRefType) func(*Context) context.CancelFunc {
 						util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), refName, ctx.Repo.Commit.ID.String(), 1))))
 				}
 			} else {
+				if len(ignoreNotExistErr) > 0 && ignoreNotExistErr[0] {
+					return
+				}
 				ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName))
 				return
 			}

routers/repo/release.go

@@ -99,7 +99,7 @@ func releasesOrTags(ctx *context.Context, isTagList bool) {
 			Page:     ctx.QueryInt("page"),
 			PageSize: convert.ToCorrectPageSize(ctx.QueryInt("limit")),
 		},
-		IncludeDrafts: writeAccess,
+		IncludeDrafts: writeAccess && !isTagList,
 		IncludeTags:   isTagList,
 	}
 
@@ -141,10 +141,7 @@ func releasesOrTags(ctx *context.Context, isTagList bool) {
 			}
 			cacheUsers[r.PublisherID] = r.Publisher
 		}
-		if err := calReleaseNumCommitsBehind(ctx.Repo, r, countCache); err != nil {
-			ctx.ServerError("calReleaseNumCommitsBehind", err)
-			return
-		}
+
 		r.Note, err = markdown.RenderString(&markup.RenderContext{
 			URLPrefix: ctx.Repo.RepoLink,
 			Metas:     ctx.Repo.Repository.ComposeMetas(),
@@ -153,6 +150,15 @@ func releasesOrTags(ctx *context.Context, isTagList bool) {
 			ctx.ServerError("RenderString", err)
 			return
 		}
+
+		if r.IsDraft {
+			continue
+		}
+
+		if err := calReleaseNumCommitsBehind(ctx.Repo, r, countCache); err != nil {
+			ctx.ServerError("calReleaseNumCommitsBehind", err)
+			return
+		}
 	}
 
 	ctx.Data["Releases"] = releases
@@ -198,9 +204,11 @@ func SingleRelease(ctx *context.Context) {
 			return
 		}
 	}
-	if err := calReleaseNumCommitsBehind(ctx.Repo, release, make(map[string]int64)); err != nil {
-		ctx.ServerError("calReleaseNumCommitsBehind", err)
-		return
+	if !release.IsDraft {
+		if err := calReleaseNumCommitsBehind(ctx.Repo, release, make(map[string]int64)); err != nil {
+			ctx.ServerError("calReleaseNumCommitsBehind", err)
+			return
+		}
 	}
 	release.Note, err = markdown.RenderString(&markup.RenderContext{
 		URLPrefix: ctx.Repo.RepoLink,

routers/routes/web.go

@@ -912,8 +912,8 @@ func RegisterRoutes(m *web.Route) {
 			m.Get("/", repo.Releases)
 			m.Get("/tag/*", repo.SingleRelease)
 			m.Get("/latest", repo.LatestRelease)
-			m.Get("/attachments/{uuid}", repo.GetAttachment)
-		}, repo.MustBeNotEmpty, reqRepoReleaseReader, context.RepoRefByType(context.RepoRefTag))
+		}, repo.MustBeNotEmpty, reqRepoReleaseReader, context.RepoRefByType(context.RepoRefTag, true))
+		m.Get("/releases/attachments/{uuid}", repo.GetAttachment, repo.MustBeNotEmpty, reqRepoReleaseReader)
 		m.Group("/releases", func() {
 			m.Get("/new", repo.NewRelease)
 			m.Post("/new", bindIgnErr(forms.NewReleaseForm{}), repo.NewReleasePost)

templates/repo/branch_dropdown.tmpl

@@ -22,7 +22,7 @@
 			{{end}}
 			{{range .root.Tags}}
 				{{if $release}}
-					<div class="item tag {{if eq $release.TagName .}}selected{{end}}" data-url="{{$.root.RepoLink}}/compare/{{EscapePound .}}...{{if $release.TagName}}{{EscapePound $release.TagName}}{{else}}{{EscapePound $release.Sha1}}{{end}}">{{.}}</div>
+					<div class="item tag {{if eq $release.TagName .}}selected{{end}}" data-url="{{$.root.RepoLink}}/compare/{{EscapePound .}}...{{if $release.IsDraft}}{{EscapePound $release.Target}}{{else}}}{{if $release.TagName}}{{EscapePound $release.TagName}}{{else}}{{EscapePound $release.Sha1}}{{end}}{{end}}">{{.}}</div>
 				{{else}}
 					<div class="item tag {{if eq $.root.BranchName .}}selected{{end}}" data-url="{{$.root.RepoLink}}/{{if $.root.PageIsCommits}}commits{{else}}src{{end}}/tag/{{EscapePound .}}{{if $.root.TreePath}}/{{EscapePound $.root.TreePath}}{{end}}">{{.}}</div>
 				{{end}}

templates/repo/release/list.tmpl

@@ -75,11 +75,13 @@
 								<span class="ui green label">{{$.i18n.Tr "repo.release.stable"}}</span>
 							{{end}}
 							<span class="tag text blue">
-								<a class="df ac je" href="{{$.RepoLink}}/src/tag/{{.TagName | EscapePound}}" rel="nofollow">{{svg "octicon-tag" 16 "mr-2"}}{{.TagName}}</a>
-							</span>
-							<span class="commit">
-								<a class="mono" href="{{$.RepoLink}}/src/commit/{{.Sha1}}" rel="nofollow">{{svg "octicon-git-commit" 16 "mr-2"}}{{ShortSha .Sha1}}</a>
+								<a class="df ac je" href="{{if .IsDraft}}#{{else}}{{$.RepoLink}}/src/tag/{{.TagName | EscapePound}}{{end}}" rel="nofollow">{{svg "octicon-tag" 16 "mr-2"}}{{.TagName}}</a>
 							</span>
+							{{if not .IsDraft}}
+								<span class="commit">
+									<a class="mono" href="{{$.RepoLink}}/src/commit/{{.Sha1}}" rel="nofollow">{{svg "octicon-git-commit" 16 "mr-2"}}{{ShortSha .Sha1}}</a>
+								</span>
+							{{end}}
 							{{template "repo/branch_dropdown" dict "root" $ "release" .}}
 						{{end}}
 					</div>
@@ -128,9 +130,11 @@
 									{{$.i18n.Tr "repo.released_this"}}
 								</span>
 								{{if .CreatedUnix}}
-									<span class="time">{{TimeSinceUnix .CreatedUnix $.Lang}}</span> |
+									<span class="time">{{TimeSinceUnix .CreatedUnix $.Lang}}</span>
+								{{end}}
+								{{if not .IsDraft}}
+									| <span class="ahead"><a href="{{$.RepoLink}}/compare/{{.TagName | EscapePound}}...{{.Target}}">{{$.i18n.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}}</a> {{$.i18n.Tr "repo.release.ahead.target" .Target}}</span>
 								{{end}}
-								<span class="ahead"><a href="{{$.RepoLink}}/compare/{{.TagName | EscapePound}}...{{.Target}}">{{$.i18n.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}}</a> {{$.i18n.Tr "repo.release.ahead.target" .Target}}</span>
 							</p>
 							<div class="markdown desc">
 								{{Str2html .Note}}
@@ -142,7 +146,7 @@
 								</h2>
 								<div class="content {{if eq $idx 0}}active{{end}}">
 									<ul class="list">
-										{{if $.Permission.CanRead $.UnitTypeCode}}
+										{{if and (not .IsDraft) ($.Permission.CanRead $.UnitTypeCode)}}
 											<li>
 												<a class="archive-link" data-url="{{$.RepoLink}}/archive/{{.TagName | EscapePound}}.zip" rel="nofollow"><strong>{{svg "octicon-file-zip" 16 "mr-2"}}{{$.i18n.Tr "repo.release.source_code"}} (ZIP)</strong></a>
 											</li>