From 783b1967e257fdba2bd593d9f2108da2010b7448 Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Thu, 29 Jun 2017 18:11:38 +0300 Subject: [PATCH] Fix release display and correct paging (#2080) --- integrations/release_test.go | 93 ++++++++++++++++++++++++++++++++++ models/fixtures/release.yml | 1 + models/release.go | 48 ++++++++++-------- routers/api/v1/repo/release.go | 19 ++++--- routers/repo/release.go | 15 +++--- 5 files changed, 136 insertions(+), 40 deletions(-) create mode 100644 models/fixtures/release.yml diff --git a/integrations/release_test.go b/integrations/release_test.go index f097531a35..43c2839638 100644 --- a/integrations/release_test.go +++ b/integrations/release_test.go @@ -5,12 +5,60 @@ package integrations import ( + "fmt" "net/http" "testing" + "github.com/Unknwon/i18n" "github.com/stretchr/testify/assert" ) +func createNewRelease(t *testing.T, session *TestSession, repoURL, tag, title string, preRelease, draft bool) { + req := NewRequest(t, "GET", repoURL+"/releases/new") + resp := session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + htmlDoc := NewHTMLParser(t, resp.Body) + + link, exists := htmlDoc.doc.Find("form").Attr("action") + assert.True(t, exists, "The template has changed") + + postData := map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "tag_name": tag, + "tag_target": "master", + "title": title, + "content": "", + } + if preRelease { + postData["prerelease"] = "on" + } + if draft { + postData["draft"] = "Save Draft" + } + req = NewRequestWithValues(t, "POST", link, postData) + + resp = session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusFound, resp.HeaderCode) + + redirectedURL := resp.Headers["Location"] + assert.NotEmpty(t, redirectedURL, "Redirected URL is not found") +} + +func checkLatestReleaseAndCount(t *testing.T, session *TestSession, repoURL, version, label string, count int) { + req := NewRequest(t, "GET", repoURL+"/releases") + resp := session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + + htmlDoc := NewHTMLParser(t, resp.Body) + labelText := htmlDoc.doc.Find("#release-list > li .meta .label").First().Text() + assert.EqualValues(t, label, labelText) + titleText := htmlDoc.doc.Find("#release-list > li .detail h3 a").First().Text() + assert.EqualValues(t, version, titleText) + + releaseList := htmlDoc.doc.Find("#release-list > li") + assert.EqualValues(t, count, releaseList.Length()) +} + func TestViewReleases(t *testing.T) { prepareTestEnv(t) @@ -27,3 +75,48 @@ func TestViewReleasesNoLogin(t *testing.T) { resp := MakeRequest(req) assert.EqualValues(t, http.StatusOK, resp.HeaderCode) } + +func TestCreateRelease(t *testing.T) { + prepareTestEnv(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"), 1) +} + +func TestCreateReleasePreRelease(t *testing.T) { + prepareTestEnv(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"), 1) +} + +func TestCreateReleaseDraft(t *testing.T) { + prepareTestEnv(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"), 1) +} + +func TestCreateReleasePaging(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user2") + // Create enaugh releases to have paging + for i := 0; i < 12; i++ { + version := fmt.Sprintf("v0.0.%d", i) + createNewRelease(t, session, "/user2/repo1", version, version, false, false) + } + createNewRelease(t, session, "/user2/repo1", "v0.0.12", "v0.0.12", false, true) + + checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.12", i18n.Tr("en", "repo.release.draft"), 10) + + // Check that user3 does not see draft and still see 10 latest releases + session2 := loginUser(t, "user3") + checkLatestReleaseAndCount(t, session2, "/user2/repo1", "v0.0.11", i18n.Tr("en", "repo.release.stable"), 10) +} diff --git a/models/fixtures/release.yml b/models/fixtures/release.yml new file mode 100644 index 0000000000..ca780a73aa --- /dev/null +++ b/models/fixtures/release.yml @@ -0,0 +1 @@ +[] # empty diff --git a/models/release.go b/models/release.go index 783f1f0475..f12fc06840 100644 --- a/models/release.go +++ b/models/release.go @@ -233,38 +233,42 @@ func GetReleaseByID(id int64) (*Release, error) { return rel, nil } +// FindReleasesOptions describes the conditions to Find releases +type FindReleasesOptions struct { + IncludeDrafts bool + TagNames []string +} + +func (opts *FindReleasesOptions) toConds(repoID int64) builder.Cond { + var cond = builder.NewCond() + cond = cond.And(builder.Eq{"repo_id": repoID}) + + if !opts.IncludeDrafts { + cond = cond.And(builder.Eq{"is_draft": false}) + } + if len(opts.TagNames) > 0 { + cond = cond.And(builder.In("tag_name", opts.TagNames)) + } + return cond +} + // GetReleasesByRepoID returns a list of releases of repository. -func GetReleasesByRepoID(repoID int64, page, pageSize int) (rels []*Release, err error) { +func GetReleasesByRepoID(repoID int64, opts FindReleasesOptions, page, pageSize int) (rels []*Release, err error) { if page <= 0 { page = 1 } + err = x. - Desc("created_unix"). + Desc("created_unix", "id"). Limit(pageSize, (page-1)*pageSize). - Find(&rels, Release{RepoID: repoID}) + Where(opts.toConds(repoID)). + Find(&rels) return rels, err } // GetReleaseCountByRepoID returns the count of releases of repository -func GetReleaseCountByRepoID(repoID int64, includeDrafts bool) (int64, error) { - var cond = builder.NewCond() - cond = cond.And(builder.Eq{"repo_id": repoID}) - - if includeDrafts { - return x.Where(cond).Count(&Release{}) - } - - cond = cond.And(builder.Eq{"is_draft": false}) - return x.Where(cond).Count(&Release{}) -} - -// GetReleasesByRepoIDAndNames returns a list of releases of repository according repoID and tagNames. -func GetReleasesByRepoIDAndNames(repoID int64, tagNames []string) (rels []*Release, err error) { - err = x. - Desc("created_unix"). - In("tag_name", tagNames). - Find(&rels, Release{RepoID: repoID}) - return rels, err +func GetReleaseCountByRepoID(repoID int64, opts FindReleasesOptions) (int64, error) { + return x.Where(opts.toConds(repoID)).Count(&Release{}) } type releaseMetaSearch struct { diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go index a367e55717..ed5b8f4f78 100644 --- a/routers/api/v1/repo/release.go +++ b/routers/api/v1/repo/release.go @@ -34,22 +34,21 @@ func GetRelease(ctx *context.APIContext) { // ListReleases list a repository's releases func ListReleases(ctx *context.APIContext) { - releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID, 1, 2147483647) - if err != nil { - ctx.Error(500, "GetReleasesByRepoID", err) - return - } - rels := make([]*api.Release, len(releases)) access, err := models.AccessLevel(ctx.User.ID, ctx.Repo.Repository) if err != nil { ctx.Error(500, "AccessLevel", err) return } + + releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID, models.FindReleasesOptions{ + IncludeDrafts: access >= models.AccessModeWrite, + }, 1, 2147483647) + if err != nil { + ctx.Error(500, "GetReleasesByRepoID", err) + return + } + rels := make([]*api.Release, len(releases)) for i, release := range releases { - if release.IsDraft && access < models.AccessModeWrite { - // hide drafts from users without push access - continue - } if err := release.LoadAttributes(); err != nil { ctx.Error(500, "LoadAttributes", err) return diff --git a/routers/repo/release.go b/routers/repo/release.go index 1b2c630a56..d9cc967801 100644 --- a/routers/repo/release.go +++ b/routers/repo/release.go @@ -65,13 +65,17 @@ func Releases(ctx *context.Context) { limit = 10 } - releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID, page, limit) + opts := models.FindReleasesOptions{ + IncludeDrafts: ctx.Repo.IsWriter(), + } + + releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID, opts, page, limit) if err != nil { ctx.Handle(500, "GetReleasesByRepoID", err) return } - count, err := models.GetReleaseCountByRepoID(ctx.Repo.Repository.ID, ctx.Repo.IsOwner()) + count, err := models.GetReleaseCountByRepoID(ctx.Repo.Repository.ID, opts) if err != nil { ctx.Handle(500, "GetReleaseCountByRepoID", err) return @@ -91,11 +95,7 @@ func Releases(ctx *context.Context) { } var ok bool - releasesToDisplay := make([]*models.Release, 0, len(releases)) for _, r := range releases { - if r.IsDraft && !ctx.Repo.IsOwner() { - continue - } if r.Publisher, ok = cacheUsers[r.PublisherID]; !ok { r.Publisher, err = models.GetUserByID(r.PublisherID) if err != nil { @@ -113,12 +113,11 @@ func Releases(ctx *context.Context) { return } r.Note = markdown.RenderString(r.Note, ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas()) - releasesToDisplay = append(releasesToDisplay, r) } pager := paginater.New(int(count), limit, page, 5) ctx.Data["Page"] = pager - ctx.Data["Releases"] = releasesToDisplay + ctx.Data["Releases"] = releases ctx.HTML(200, tplReleases) }