diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go
index 53655703fc..8909dedbb1 100644
--- a/routers/web/repo/release.go
+++ b/routers/web/repo/release.go
@@ -17,7 +17,6 @@ import (
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
- "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
@@ -330,34 +329,17 @@ func LatestRelease(ctx *context.Context) {
ctx.Redirect(release.Link())
}
-// NewRelease render creating or edit release page
-func NewRelease(ctx *context.Context) {
+func newReleaseCommon(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.release.new_release")
ctx.Data["PageIsReleaseList"] = true
- ctx.Data["tag_target"] = ctx.Repo.Repository.DefaultBranch
- if tagName := ctx.FormString("tag"); len(tagName) > 0 {
- rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, tagName)
- if err != nil && !repo_model.IsErrReleaseNotExist(err) {
- ctx.ServerError("GetRelease", err)
- return
- }
- if rel != nil {
- rel.Repo = ctx.Repo.Repository
- if err := rel.LoadAttributes(ctx); err != nil {
- ctx.ServerError("LoadAttributes", err)
- return
- }
-
- ctx.Data["tag_name"] = rel.TagName
- if rel.Target != "" {
- ctx.Data["tag_target"] = rel.Target
- }
- ctx.Data["title"] = rel.Title
- ctx.Data["content"] = rel.Note
- ctx.Data["attachments"] = rel.Attachments
- }
+ tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID)
+ if err != nil {
+ ctx.ServerError("GetTagNamesByRepoID", err)
+ return
}
+ ctx.Data["Tags"] = tags
+
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
assigneeUsers, err := repo_model.GetRepoAssignees(ctx, ctx.Repo.Repository)
if err != nil {
@@ -368,35 +350,74 @@ func NewRelease(ctx *context.Context) {
upload.AddUploadContext(ctx, "release")
- // For New Release page
- PrepareBranchList(ctx)
+ PrepareBranchList(ctx) // for New Release page
+}
+
+// NewRelease render creating or edit release page
+func NewRelease(ctx *context.Context) {
+ newReleaseCommon(ctx)
if ctx.Written() {
return
}
- tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID)
- if err != nil {
- ctx.ServerError("GetTagNamesByRepoID", err)
- return
+ ctx.Data["ShowCreateTagOnlyButton"] = true
+
+ // pre-fill the form with the tag name, target branch and the existing release (if exists)
+ ctx.Data["tag_target"] = ctx.Repo.Repository.DefaultBranch
+ if tagName := ctx.FormString("tag"); tagName != "" {
+ rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, tagName)
+ if err != nil && !repo_model.IsErrReleaseNotExist(err) {
+ ctx.ServerError("GetRelease", err)
+ return
+ }
+
+ if rel != nil {
+ rel.Repo = ctx.Repo.Repository
+ if err = rel.LoadAttributes(ctx); err != nil {
+ ctx.ServerError("LoadAttributes", err)
+ return
+ }
+
+ ctx.Data["ShowCreateTagOnlyButton"] = false
+ ctx.Data["tag_name"] = rel.TagName
+ ctx.Data["tag_target"] = rel.Target
+ ctx.Data["title"] = rel.Title
+ ctx.Data["content"] = rel.Note
+ ctx.Data["attachments"] = rel.Attachments
+ }
}
- ctx.Data["Tags"] = tags
ctx.HTML(http.StatusOK, tplReleaseNew)
}
// NewReleasePost response for creating a release
func NewReleasePost(ctx *context.Context) {
- form := web.GetForm(ctx).(*forms.NewReleaseForm)
- ctx.Data["Title"] = ctx.Tr("repo.release.new_release")
- ctx.Data["PageIsReleaseList"] = true
-
- tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID)
- if err != nil {
- ctx.ServerError("GetTagNamesByRepoID", err)
+ newReleaseCommon(ctx)
+ if ctx.Written() {
return
}
- ctx.Data["Tags"] = tags
+ form := web.GetForm(ctx).(*forms.NewReleaseForm)
+
+ // first, check whether the release exists, and prepare "ShowCreateTagOnlyButton"
+ // the logic should be done before the form error check to make the tmpl has correct variables
+ rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, form.TagName)
+ if err != nil && !repo_model.IsErrReleaseNotExist(err) {
+ ctx.ServerError("GetRelease", err)
+ return
+ }
+
+ // We should still show the "tag only" button if the user clicks it, no matter the release exists or not.
+ // Because if error occurs, end users need to have the chance to edit the name and submit the form with "tag-only" again.
+ // It is still not completely right, because there could still be cases like this:
+ // * user visit "new release" page, see the "tag only" button
+ // * input something, click other buttons but not "tag only"
+ // * error occurs, the "new release" page is rendered again, but the "tag only" button is gone
+ // Such cases are not able to be handled by current code, it needs frontend code to toggle the "tag-only" button if the input changes.
+ // Or another choice is "always show the tag-only button" if error occurs.
+ ctx.Data["ShowCreateTagOnlyButton"] = form.TagOnly || rel == nil
+
+ // do some form checks
if ctx.HasError() {
ctx.HTML(http.StatusOK, tplReleaseNew)
return
@@ -407,59 +428,49 @@ func NewReleasePost(ctx *context.Context) {
return
}
- // Title of release cannot be empty
- if len(form.TagOnly) == 0 && len(form.Title) == 0 {
+ if !form.TagOnly && form.Title == "" {
+ // if not "tag only", then the title of the release cannot be empty
ctx.RenderWithErr(ctx.Tr("repo.release.title_empty"), tplReleaseNew, &form)
return
}
- var attachmentUUIDs []string
- if setting.Attachment.Enabled {
- attachmentUUIDs = form.Files
+ handleTagReleaseError := func(err error) {
+ ctx.Data["Err_TagName"] = true
+ switch {
+ case release_service.IsErrTagAlreadyExists(err):
+ ctx.RenderWithErr(ctx.Tr("repo.branch.tag_collision", form.TagName), tplReleaseNew, &form)
+ case repo_model.IsErrReleaseAlreadyExist(err):
+ ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form)
+ case release_service.IsErrInvalidTagName(err):
+ ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_invalid"), tplReleaseNew, &form)
+ case release_service.IsErrProtectedTagName(err):
+ ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_protected"), tplReleaseNew, &form)
+ default:
+ ctx.ServerError("handleTagReleaseError", err)
+ }
}
- rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, form.TagName)
- if err != nil {
- if !repo_model.IsErrReleaseNotExist(err) {
- ctx.ServerError("GetRelease", err)
+ // prepare the git message for creating a new tag
+ newTagMsg := ""
+ if form.Title != "" && form.AddTagMsg {
+ newTagMsg = form.Title + "\n\n" + form.Content
+ }
+
+ // no release, and tag only
+ if rel == nil && form.TagOnly {
+ if err = release_service.CreateNewTag(ctx, ctx.Doer, ctx.Repo.Repository, form.Target, form.TagName, newTagMsg); err != nil {
+ handleTagReleaseError(err)
return
}
+ ctx.Flash.Success(ctx.Tr("repo.tag.create_success", form.TagName))
+ ctx.Redirect(ctx.Repo.RepoLink + "/src/tag/" + util.PathEscapeSegments(form.TagName))
+ return
+ }
- msg := ""
- if len(form.Title) > 0 && form.AddTagMsg {
- msg = form.Title + "\n\n" + form.Content
- }
-
- if len(form.TagOnly) > 0 {
- if err = release_service.CreateNewTag(ctx, ctx.Doer, ctx.Repo.Repository, form.Target, form.TagName, msg); err != nil {
- if release_service.IsErrTagAlreadyExists(err) {
- e := err.(release_service.ErrTagAlreadyExists)
- ctx.Flash.Error(ctx.Tr("repo.branch.tag_collision", e.TagName))
- ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.RefTypeNameSubURL())
- return
- }
-
- if release_service.IsErrInvalidTagName(err) {
- ctx.Flash.Error(ctx.Tr("repo.release.tag_name_invalid"))
- ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.RefTypeNameSubURL())
- return
- }
-
- if release_service.IsErrProtectedTagName(err) {
- ctx.Flash.Error(ctx.Tr("repo.release.tag_name_protected"))
- ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.RefTypeNameSubURL())
- return
- }
-
- ctx.ServerError("release_service.CreateNewTag", err)
- return
- }
-
- ctx.Flash.Success(ctx.Tr("repo.tag.create_success", form.TagName))
- ctx.Redirect(ctx.Repo.RepoLink + "/src/tag/" + util.PathEscapeSegments(form.TagName))
- return
- }
+ attachmentUUIDs := util.Iif(setting.Attachment.Enabled, form.Files, nil)
+ // no existing release, create a new release
+ if rel == nil {
rel = &repo_model.Release{
RepoID: ctx.Repo.Repository.ID,
Repo: ctx.Repo.Repository,
@@ -469,48 +480,39 @@ func NewReleasePost(ctx *context.Context) {
TagName: form.TagName,
Target: form.Target,
Note: form.Content,
- IsDraft: len(form.Draft) > 0,
+ IsDraft: form.Draft,
IsPrerelease: form.Prerelease,
IsTag: false,
}
-
- if err = release_service.CreateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs, msg); err != nil {
- ctx.Data["Err_TagName"] = true
- switch {
- case repo_model.IsErrReleaseAlreadyExist(err):
- ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form)
- case release_service.IsErrInvalidTagName(err):
- ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_invalid"), tplReleaseNew, &form)
- case release_service.IsErrProtectedTagName(err):
- ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_protected"), tplReleaseNew, &form)
- default:
- ctx.ServerError("CreateRelease", err)
- }
- return
- }
- } else {
- if !rel.IsTag {
- ctx.Data["Err_TagName"] = true
- ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form)
- return
- }
-
- rel.Title = form.Title
- rel.Note = form.Content
- rel.Target = form.Target
- rel.IsDraft = len(form.Draft) > 0
- rel.IsPrerelease = form.Prerelease
- rel.PublisherID = ctx.Doer.ID
- rel.IsTag = false
-
- if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil); err != nil {
- ctx.Data["Err_TagName"] = true
- ctx.ServerError("UpdateRelease", err)
+ if err = release_service.CreateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs, newTagMsg); err != nil {
+ handleTagReleaseError(err)
return
}
+ ctx.Redirect(ctx.Repo.RepoLink + "/releases")
+ return
}
- log.Trace("Release created: %s/%s:%s", ctx.Doer.LowerName, ctx.Repo.Repository.Name, form.TagName)
+ // tag exists, try to convert it to a real release
+ // old logic: if the release is not a tag (it is a real release), do not update it on the "new release" page
+ // add new logic: if tag-only, do not convert the tag to a release
+ if form.TagOnly || !rel.IsTag {
+ ctx.Data["Err_TagName"] = true
+ ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form)
+ return
+ }
+
+ // convert a tag to a real release (set is_tag=false)
+ rel.Title = form.Title
+ rel.Note = form.Content
+ rel.Target = form.Target
+ rel.IsDraft = form.Draft
+ rel.IsPrerelease = form.Prerelease
+ rel.PublisherID = ctx.Doer.ID
+ rel.IsTag = false
+ if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil); err != nil {
+ handleTagReleaseError(err)
+ return
+ }
ctx.Redirect(ctx.Repo.RepoLink + "/releases")
}
diff --git a/routers/web/repo/release_test.go b/routers/web/repo/release_test.go
index 7ebea4c3fb..9f49fc7500 100644
--- a/routers/web/repo/release_test.go
+++ b/routers/web/repo/release_test.go
@@ -11,60 +11,135 @@ import (
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/web"
+ "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/contexttest"
"code.gitea.io/gitea/services/forms"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestNewReleasePost(t *testing.T) {
- for _, testCase := range []struct {
- RepoID int64
- UserID int64
- TagName string
- Form forms.NewReleaseForm
- }{
- {
- RepoID: 1,
- UserID: 2,
- TagName: "v1.1", // pre-existing tag
- Form: forms.NewReleaseForm{
- TagName: "newtag",
- Target: "master",
- Title: "title",
- Content: "content",
- },
- },
- {
- RepoID: 1,
- UserID: 2,
- TagName: "newtag",
- Form: forms.NewReleaseForm{
- TagName: "newtag",
- Target: "master",
- Title: "title",
- Content: "content",
- },
- },
- } {
- unittest.PrepareTestEnv(t)
+ unittest.PrepareTestEnv(t)
+ get := func(t *testing.T, tagName string) *context.Context {
+ ctx, _ := contexttest.MockContext(t, "user2/repo1/releases/new?tag="+tagName)
+ contexttest.LoadUser(t, ctx, 2)
+ contexttest.LoadRepo(t, ctx, 1)
+ contexttest.LoadGitRepo(t, ctx)
+ defer ctx.Repo.GitRepo.Close()
+ NewRelease(ctx)
+ return ctx
+ }
+
+ t.Run("NewReleasePage", func(t *testing.T) {
+ ctx := get(t, "v1.1")
+ assert.Empty(t, ctx.Data["ShowCreateTagOnlyButton"])
+ ctx = get(t, "new-tag-name")
+ assert.NotEmpty(t, ctx.Data["ShowCreateTagOnlyButton"])
+ })
+
+ post := func(t *testing.T, form forms.NewReleaseForm) *context.Context {
ctx, _ := contexttest.MockContext(t, "user2/repo1/releases/new")
contexttest.LoadUser(t, ctx, 2)
contexttest.LoadRepo(t, ctx, 1)
contexttest.LoadGitRepo(t, ctx)
- web.SetForm(ctx, &testCase.Form)
+ defer ctx.Repo.GitRepo.Close()
+ web.SetForm(ctx, &form)
NewReleasePost(ctx)
- unittest.AssertExistsAndLoadBean(t, &repo_model.Release{
- RepoID: 1,
- PublisherID: 2,
- TagName: testCase.Form.TagName,
- Target: testCase.Form.Target,
- Title: testCase.Form.Title,
- Note: testCase.Form.Content,
- }, unittest.Cond("is_draft=?", len(testCase.Form.Draft) > 0))
- ctx.Repo.GitRepo.Close()
+ return ctx
}
+
+ loadRelease := func(t *testing.T, tagName string) *repo_model.Release {
+ return unittest.GetBean(t, &repo_model.Release{}, unittest.Cond("repo_id=1 AND tag_name=?", tagName))
+ }
+
+ t.Run("NewTagRelease", func(t *testing.T) {
+ post(t, forms.NewReleaseForm{
+ TagName: "newtag",
+ Target: "master",
+ Title: "title",
+ Content: "content",
+ })
+ rel := loadRelease(t, "newtag")
+ require.NotNil(t, rel)
+ assert.False(t, rel.IsTag)
+ assert.Equal(t, "master", rel.Target)
+ assert.Equal(t, "title", rel.Title)
+ assert.Equal(t, "content", rel.Note)
+ })
+
+ t.Run("ReleaseExistsDoUpdate(non-tag)", func(t *testing.T) {
+ ctx := post(t, forms.NewReleaseForm{
+ TagName: "v1.1",
+ Target: "master",
+ Title: "updated-title",
+ Content: "updated-content",
+ })
+ rel := loadRelease(t, "v1.1")
+ require.NotNil(t, rel)
+ assert.False(t, rel.IsTag)
+ assert.Equal(t, "testing-release", rel.Title)
+ assert.NotEmpty(t, ctx.Flash.ErrorMsg)
+ })
+
+ t.Run("ReleaseExistsDoUpdate(tag-only)", func(t *testing.T) {
+ ctx := post(t, forms.NewReleaseForm{
+ TagName: "delete-tag", // a strange name, but it is the only "is_tag=true" fixture
+ Target: "master",
+ Title: "updated-title",
+ Content: "updated-content",
+ TagOnly: true,
+ })
+ rel := loadRelease(t, "delete-tag")
+ require.NotNil(t, rel)
+ assert.True(t, rel.IsTag) // the record should not be updated because the request is "tag-only". TODO: need to improve the logic?
+ assert.Equal(t, "delete-tag", rel.Title)
+ assert.NotEmpty(t, ctx.Flash.ErrorMsg)
+ assert.NotEmpty(t, ctx.Data["ShowCreateTagOnlyButton"]) // still show the "tag-only" button
+ })
+
+ t.Run("ReleaseExistsDoUpdate(tag-release)", func(t *testing.T) {
+ ctx := post(t, forms.NewReleaseForm{
+ TagName: "delete-tag", // a strange name, but it is the only "is_tag=true" fixture
+ Target: "master",
+ Title: "updated-title",
+ Content: "updated-content",
+ })
+ rel := loadRelease(t, "delete-tag")
+ require.NotNil(t, rel)
+ assert.False(t, rel.IsTag) // the tag has been "updated" to be a real "release"
+ assert.Equal(t, "updated-title", rel.Title)
+ assert.Empty(t, ctx.Flash.ErrorMsg)
+ })
+
+ t.Run("TagOnly", func(t *testing.T) {
+ ctx := post(t, forms.NewReleaseForm{
+ TagName: "new-tag-only",
+ Target: "master",
+ Title: "title",
+ Content: "content",
+ TagOnly: true,
+ })
+ rel := loadRelease(t, "new-tag-only")
+ require.NotNil(t, rel)
+ assert.True(t, rel.IsTag)
+ assert.Empty(t, ctx.Flash.ErrorMsg)
+ })
+
+ t.Run("TagOnlyConflict", func(t *testing.T) {
+ ctx := post(t, forms.NewReleaseForm{
+ TagName: "v1.1",
+ Target: "master",
+ Title: "title",
+ Content: "content",
+ TagOnly: true,
+ })
+ rel := loadRelease(t, "v1.1")
+ require.NotNil(t, rel)
+ assert.False(t, rel.IsTag)
+ assert.NotEmpty(t, ctx.Flash.ErrorMsg)
+ })
}
func TestCalReleaseNumCommitsBehind(t *testing.T) {
diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go
index b14171787e..35ea5378d3 100644
--- a/services/forms/repo_form.go
+++ b/services/forms/repo_form.go
@@ -651,8 +651,8 @@ type NewReleaseForm struct {
Target string `form:"tag_target" binding:"Required;MaxSize(255)"`
Title string `binding:"MaxSize(255)"`
Content string
- Draft string
- TagOnly string
+ Draft bool
+ TagOnly bool
Prerelease bool
AddTagMsg bool
Files []string
diff --git a/templates/repo/release/new.tmpl b/templates/repo/release/new.tmpl
index 574b0d0311..8b6aa252af 100644
--- a/templates/repo/release/new.tmpl
+++ b/templates/repo/release/new.tmpl
@@ -109,23 +109,17 @@
{{ctx.Locale.Tr "repo.release.delete_release"}}
</a>
{{if .IsDraft}}
- <button class="ui small button" type="submit" name="draft" value="{{ctx.Locale.Tr "repo.release.save_draft"}}">{{ctx.Locale.Tr "repo.release.save_draft"}}</button>
- <button class="ui small primary button">
- {{ctx.Locale.Tr "repo.release.publish"}}
- </button>
+ <button class="ui small button" type="submit" name="draft" value="1">{{ctx.Locale.Tr "repo.release.save_draft"}}</button>
+ <button class="ui small primary button">{{ctx.Locale.Tr "repo.release.publish"}}</button>
{{else}}
- <button class="ui small primary button">
- {{ctx.Locale.Tr "repo.release.edit_release"}}
- </button>
+ <button class="ui small primary button">{{ctx.Locale.Tr "repo.release.edit_release"}}</button>
{{end}}
{{else}}
- {{if not .tag_name}}
+ {{if .ShowCreateTagOnlyButton}}
<button class="ui small button" name="tag_only" value="1">{{ctx.Locale.Tr "repo.release.add_tag"}}</button>
{{end}}
<button class="ui small button" name="draft" value="1">{{ctx.Locale.Tr "repo.release.save_draft"}}</button>
- <button class="ui small primary button">
- {{ctx.Locale.Tr "repo.release.publish"}}
- </button>
+ <button class="ui small primary button">{{ctx.Locale.Tr "repo.release.publish"}}</button>
{{end}}
</div>
</div>
diff --git a/tests/integration/release_test.go b/tests/integration/release_test.go
index 40bd798d16..d3c4ed6a83 100644
--- a/tests/integration/release_test.go
+++ b/tests/integration/release_test.go
@@ -39,7 +39,7 @@ func createNewRelease(t *testing.T, session *TestSession, repoURL, tag, title st
postData["prerelease"] = "on"
}
if draft {
- postData["draft"] = "Save Draft"
+ postData["draft"] = "1"
}
req = NewRequestWithValues(t, "POST", link, postData)