From a83fb3a83aad5cb08b8ab0692e4fc74704025425 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Fri, 7 May 2021 03:23:26 +0800 Subject: [PATCH] fix some ui bug about draft release (#15137) (#15745) * 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 | 15 ++++-- routers/routes/web.go | 4 +- templates/repo/release/list.tmpl | 18 ++++--- 6 files changed, 122 insertions(+), 17 deletions(-) diff --git a/integrations/release_test.go b/integrations/release_test.go index a14ad8434..365bc04d8 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 8d3f5840e..5e577d3fd 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 5ce31e9e3..a77d075de 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -722,7 +722,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) { +func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context) { return func(ctx *Context) { // Empty repository does not have reference information. if ctx.Repo.Repository.IsEmpty { @@ -811,6 +811,9 @@ func RepoRefByType(refType RepoRefType) func(*Context) { 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 7c87fce32..5d12e8d93 100644 --- a/routers/repo/release.go +++ b/routers/repo/release.go @@ -85,7 +85,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, } @@ -127,6 +127,11 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { } cacheUsers[r.PublisherID] = r.Publisher } + + if r.IsDraft { + continue + } + if err := calReleaseNumCommitsBehind(ctx.Repo, r, countCache); err != nil { ctx.ServerError("calReleaseNumCommitsBehind", err) return @@ -177,9 +182,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 = markdown.RenderString(release.Note, ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas()) diff --git a/routers/routes/web.go b/routers/routes/web.go index e4ec3aa52..952168049 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -901,8 +901,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(auth.NewReleaseForm{}), repo.NewReleasePost) diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index c561a9d06..1a77c6cb0 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}} {{end}}
@@ -127,9 +129,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}} @@ -141,7 +145,7 @@