From 308886653148d859080222ebb72966e020f9391e Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Thu, 6 May 2021 11:12:50 +0800 Subject: [PATCH] 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 | 85 ++++++++++++++++++++++++++++- models/fixtures/release.yml | 12 ++++ modules/context/repo.go | 5 +- routers/repo/release.go | 24 +++++--- routers/routes/web.go | 4 +- templates/repo/branch_dropdown.tmpl | 2 +- templates/repo/release/list.tmpl | 18 +++--- 7 files changed, 128 insertions(+), 22 deletions(-) diff --git a/integrations/release_test.go b/integrations/release_test.go index a14ad8434e..365bc04d87 100644 --- a/integrations/release_test.go +++ b/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) +} diff --git a/models/fixtures/release.yml b/models/fixtures/release.yml index 8d3f5840ef..5e577d3fdd 100644 --- a/models/fixtures/release.yml +++ b/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 diff --git a/modules/context/repo.go b/modules/context/repo.go index 8fc948b509..c1f60a1362 100644 --- a/modules/context/repo.go +++ b/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 } diff --git a/routers/repo/release.go b/routers/repo/release.go index 6b0b92743d..b7730e4ee2 100644 --- a/routers/repo/release.go +++ b/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, diff --git a/routers/routes/web.go b/routers/routes/web.go index eb2a9025d0..ebd738de1e 100644 --- a/routers/routes/web.go +++ b/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) diff --git a/templates/repo/branch_dropdown.tmpl b/templates/repo/branch_dropdown.tmpl index 3fd461c64f..9ddadeb3da 100644 --- a/templates/repo/branch_dropdown.tmpl +++ b/templates/repo/branch_dropdown.tmpl @@ -22,7 +22,7 @@ {{end}} {{range .root.Tags}} {{if $release}} -
{{.}}
+
{{.}}
{{else}}
{{.}}
{{end}} diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index 6829e3e8e2..63f9e26cf1 100644 --- a/templates/repo/release/list.tmpl +++ b/templates/repo/release/list.tmpl @@ -75,11 +75,13 @@ {{$.i18n.Tr "repo.release.stable"}} {{end}} - {{svg "octicon-tag" 16 "mr-2"}}{{.TagName}} - - - {{svg "octicon-git-commit" 16 "mr-2"}}{{ShortSha .Sha1}} + {{svg "octicon-tag" 16 "mr-2"}}{{.TagName}} + {{if not .IsDraft}} + + {{svg "octicon-git-commit" 16 "mr-2"}}{{ShortSha .Sha1}} + + {{end}} {{template "repo/branch_dropdown" dict "root" $ "release" .}} {{end}} @@ -128,9 +130,11 @@ {{$.i18n.Tr "repo.released_this"}} {{if .CreatedUnix}} - {{TimeSinceUnix .CreatedUnix $.Lang}} | + {{TimeSinceUnix .CreatedUnix $.Lang}} + {{end}} + {{if not .IsDraft}} + | {{$.i18n.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}} {{$.i18n.Tr "repo.release.ahead.target" .Target}} {{end}} - {{$.i18n.Tr "repo.release.ahead.commits" .NumCommitsBehind | Str2html}} {{$.i18n.Tr "repo.release.ahead.target" .Target}}

{{Str2html .Note}} @@ -142,7 +146,7 @@