Fix release display and correct paging (#2080)

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)
+}

models/fixtures/release.yml

@@ -0,0 +1 @@
+[] # empty

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) {
+func GetReleaseCountByRepoID(repoID int64, opts FindReleasesOptions) (int64, error) {
-	var cond = builder.NewCond()
+	return x.Where(opts.toConds(repoID)).Count(&Release{})
-	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
 }
 
 type releaseMetaSearch struct {

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

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)
 }