Implement delete release attachments and update release attachments' name (#14130)

* Implement delete release attachment

* Add attachments on release edit page

* Fix bug

* Finish del release attachments

* Fix frontend lint

* Fix tests

* Support edit release attachments

* Added tests

* Remove the unnecessary parameter isCreate from UpdateReleaseOrCreatReleaseFromTag

* Rename UpdateReleaseOrCreatReleaseFromTag to UpdateRelease

* Fix middle align

11 files changed, 276 insertions, 56 deletions

diff --git a/models/attachment.go b/models/attachment.go
index 478b4cd0d2..2126e6d773 100644
--- a/models/attachment.go
+++ b/models/attachment.go
@@ -125,8 +125,8 @@ func getAttachmentByUUID(e Engine, uuid string) (*Attachment, error) {
 }
 
 // GetAttachmentsByUUIDs returns attachment by given UUID list.
-func GetAttachmentsByUUIDs(uuids []string) ([]*Attachment, error) {
-	return getAttachmentsByUUIDs(x, uuids)
+func GetAttachmentsByUUIDs(ctx DBContext, uuids []string) ([]*Attachment, error) {
+	return getAttachmentsByUUIDs(ctx.e, uuids)
 }
 
 func getAttachmentsByUUIDs(e Engine, uuids []string) ([]*Attachment, error) {
@@ -183,12 +183,12 @@ func getAttachmentByReleaseIDFileName(e Engine, releaseID int64, fileName string
 
 // DeleteAttachment deletes the given attachment and optionally the associated file.
 func DeleteAttachment(a *Attachment, remove bool) error {
-	_, err := DeleteAttachments([]*Attachment{a}, remove)
+	_, err := DeleteAttachments(DefaultDBContext(), []*Attachment{a}, remove)
 	return err
 }
 
 // DeleteAttachments deletes the given attachments and optionally the associated files.
-func DeleteAttachments(attachments []*Attachment, remove bool) (int, error) {
+func DeleteAttachments(ctx DBContext, attachments []*Attachment, remove bool) (int, error) {
 	if len(attachments) == 0 {
 		return 0, nil
 	}
@@ -198,7 +198,7 @@ func DeleteAttachments(attachments []*Attachment, remove bool) (int, error) {
 		ids = append(ids, a.ID)
 	}
 
-	cnt, err := x.In("id", ids).NoAutoCondition().Delete(attachments[0])
+	cnt, err := ctx.e.In("id", ids).NoAutoCondition().Delete(attachments[0])
 	if err != nil {
 		return 0, err
 	}
@@ -220,7 +220,7 @@ func DeleteAttachmentsByIssue(issueID int64, remove bool) (int, error) {
 		return 0, err
 	}
 
-	return DeleteAttachments(attachments, remove)
+	return DeleteAttachments(DefaultDBContext(), attachments, remove)
 }
 
 // DeleteAttachmentsByComment deletes all attachments associated with the given comment.
@@ -230,7 +230,7 @@ func DeleteAttachmentsByComment(commentID int64, remove bool) (int, error) {
 		return 0, err
 	}
 
-	return DeleteAttachments(attachments, remove)
+	return DeleteAttachments(DefaultDBContext(), attachments, remove)
 }
 
 // UpdateAttachment updates the given attachment in database
@@ -238,6 +238,15 @@ func UpdateAttachment(atta *Attachment) error {
 	return updateAttachment(x, atta)
 }
 
+// UpdateAttachmentByUUID Updates attachment via uuid
+func UpdateAttachmentByUUID(ctx DBContext, attach *Attachment, cols ...string) error {
+	if attach.UUID == "" {
+		return fmt.Errorf("Attachement uuid should not blank")
+	}
+	_, err := ctx.e.Where("uuid=?", attach.UUID).Cols(cols...).Update(attach)
+	return err
+}
+
 func updateAttachment(e Engine, atta *Attachment) error {
 	var sess *xorm.Session
 	if atta.ID != 0 && atta.UUID == "" {
diff --git a/models/attachment_test.go b/models/attachment_test.go
index e36a5977ee..fa7fd3471b 100644
--- a/models/attachment_test.go
+++ b/models/attachment_test.go
@@ -120,7 +120,7 @@ func TestUpdateAttachment(t *testing.T) {
 func TestGetAttachmentsByUUIDs(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
-	attachList, err := GetAttachmentsByUUIDs([]string{"a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", "not-existing-uuid"})
+	attachList, err := GetAttachmentsByUUIDs(DefaultDBContext(), []string{"a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a17", "not-existing-uuid"})
 	assert.NoError(t, err)
 	assert.Equal(t, 2, len(attachList))
 	assert.Equal(t, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", attachList[0].UUID)
diff --git a/models/release.go b/models/release.go
index 751d08d4d8..13b8f17218 100644
--- a/models/release.go
+++ b/models/release.go
@@ -6,6 +6,7 @@
 package models
 
 import (
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -117,17 +118,20 @@ func UpdateRelease(ctx DBContext, rel *Release) error {
 }
 
 // AddReleaseAttachments adds a release attachments
-func AddReleaseAttachments(releaseID int64, attachmentUUIDs []string) (err error) {
+func AddReleaseAttachments(ctx DBContext, releaseID int64, attachmentUUIDs []string) (err error) {
 	// Check attachments
-	attachments, err := GetAttachmentsByUUIDs(attachmentUUIDs)
+	attachments, err := getAttachmentsByUUIDs(ctx.e, attachmentUUIDs)
 	if err != nil {
 		return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %v", attachmentUUIDs, err)
 	}
 
 	for i := range attachments {
+		if attachments[i].ReleaseID != 0 {
+			return errors.New("release permission denied")
+		}
 		attachments[i].ReleaseID = releaseID
 		// No assign value could be 0, so ignore AllCols().
-		if _, err = x.ID(attachments[i].ID).Update(attachments[i]); err != nil {
+		if _, err = ctx.e.ID(attachments[i].ID).Update(attachments[i]); err != nil {
 			return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err)
 		}
 	}
diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go
index e9eb4e0f9e..327a2d790b 100644
--- a/routers/api/v1/repo/release.go
+++ b/routers/api/v1/repo/release.go
@@ -202,8 +202,8 @@ func CreateRelease(ctx *context.APIContext) {
 		rel.Repo = ctx.Repo.Repository
 		rel.Publisher = ctx.User
 
-		if err = releaseservice.UpdateReleaseOrCreatReleaseFromTag(ctx.User, ctx.Repo.GitRepo, rel, nil, true); err != nil {
-			ctx.Error(http.StatusInternalServerError, "UpdateReleaseOrCreatReleaseFromTag", err)
+		if err = releaseservice.UpdateRelease(ctx.User, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil {
+			ctx.Error(http.StatusInternalServerError, "UpdateRelease", err)
 			return
 		}
 	}
@@ -277,8 +277,8 @@ func EditRelease(ctx *context.APIContext) {
 	if form.IsPrerelease != nil {
 		rel.IsPrerelease = *form.IsPrerelease
 	}
-	if err := releaseservice.UpdateReleaseOrCreatReleaseFromTag(ctx.User, ctx.Repo.GitRepo, rel, nil, false); err != nil {
-		ctx.Error(http.StatusInternalServerError, "UpdateReleaseOrCreatReleaseFromTag", err)
+	if err := releaseservice.UpdateRelease(ctx.User, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil {
+		ctx.Error(http.StatusInternalServerError, "UpdateRelease", err)
 		return
 	}
 
diff --git a/routers/repo/release.go b/routers/repo/release.go
index d89a173669..7c87fce327 100644
--- a/routers/repo/release.go
+++ b/routers/repo/release.go
@@ -7,6 +7,7 @@ package repo
 
 import (
 	"fmt"
+	"strings"
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/base"
@@ -206,7 +207,7 @@ func LatestRelease(ctx *context.Context) {
 	ctx.Redirect(release.HTMLURL())
 }
 
-// NewRelease render creating release page
+// NewRelease render creating or edit release page
 func NewRelease(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("repo.release.new_release")
 	ctx.Data["PageIsReleaseList"] = true
@@ -221,10 +222,17 @@ func NewRelease(ctx *context.Context) {
 		}
 
 		if rel != nil {
+			rel.Repo = ctx.Repo.Repository
+			if err := rel.LoadAttributes(); err != nil {
+				ctx.ServerError("LoadAttributes", err)
+				return
+			}
+
 			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["IsAttachmentEnabled"] = setting.Attachment.Enabled
@@ -324,9 +332,9 @@ func NewReleasePost(ctx *context.Context) {
 		rel.PublisherID = ctx.User.ID
 		rel.IsTag = false
 
-		if err = releaseservice.UpdateReleaseOrCreatReleaseFromTag(ctx.User, ctx.Repo.GitRepo, rel, attachmentUUIDs, true); err != nil {
+		if err = releaseservice.UpdateRelease(ctx.User, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil); err != nil {
 			ctx.Data["Err_TagName"] = true
-			ctx.ServerError("UpdateReleaseOrCreatReleaseFromTag", err)
+			ctx.ServerError("UpdateRelease", err)
 			return
 		}
 	}
@@ -363,6 +371,13 @@ func EditRelease(ctx *context.Context) {
 	ctx.Data["prerelease"] = rel.IsPrerelease
 	ctx.Data["IsDraft"] = rel.IsDraft
 
+	rel.Repo = ctx.Repo.Repository
+	if err := rel.LoadAttributes(); err != nil {
+		ctx.ServerError("LoadAttributes", err)
+		return
+	}
+	ctx.Data["attachments"] = rel.Attachments
+
 	ctx.HTML(200, tplReleaseNew)
 }
 
@@ -400,16 +415,27 @@ func EditReleasePost(ctx *context.Context) {
 		return
 	}
 
-	var attachmentUUIDs []string
+	const delPrefix = "attachment-del-"
+	const editPrefix = "attachment-edit-"
+	var addAttachmentUUIDs, delAttachmentUUIDs []string
+	var editAttachments = make(map[string]string) // uuid -> new name
 	if setting.Attachment.Enabled {
-		attachmentUUIDs = form.Files
+		addAttachmentUUIDs = form.Files
+		for k, v := range ctx.Req.Form {
+			if strings.HasPrefix(k, delPrefix) && v[0] == "true" {
+				delAttachmentUUIDs = append(delAttachmentUUIDs, k[len(delPrefix):])
+			} else if strings.HasPrefix(k, editPrefix) {
+				editAttachments[k[len(editPrefix):]] = v[0]
+			}
+		}
 	}
 
 	rel.Title = form.Title
 	rel.Note = form.Content
 	rel.IsDraft = len(form.Draft) > 0
 	rel.IsPrerelease = form.Prerelease
-	if err = releaseservice.UpdateReleaseOrCreatReleaseFromTag(ctx.User, ctx.Repo.GitRepo, rel, attachmentUUIDs, false); err != nil {
+	if err = releaseservice.UpdateRelease(ctx.User, ctx.Repo.GitRepo,
+		rel, addAttachmentUUIDs, delAttachmentUUIDs, editAttachments); err != nil {
 		ctx.ServerError("UpdateRelease", err)
 		return
 	}
diff --git a/services/release/release.go b/services/release/release.go
index d04e538c92..9d201edf6d 100644
--- a/services/release/release.go
+++ b/services/release/release.go
@@ -5,6 +5,7 @@
 package release
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -17,13 +18,14 @@ import (
 	"code.gitea.io/gitea/modules/timeutil"
 )
 
-func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
+func createTag(gitRepo *git.Repository, rel *models.Release, msg string) (bool, error) {
+	var created bool
 	// Only actual create when publish.
 	if !rel.IsDraft {
 		if !gitRepo.IsTagExist(rel.TagName) {
 			commit, err := gitRepo.GetCommit(rel.Target)
 			if err != nil {
-				return fmt.Errorf("GetCommit: %v", err)
+				return false, fmt.Errorf("GetCommit: %v", err)
 			}
 
 			// Trim '--' prefix to prevent command line argument vulnerability.
@@ -31,25 +33,26 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
 			if len(msg) > 0 {
 				if err = gitRepo.CreateAnnotatedTag(rel.TagName, msg, commit.ID.String()); err != nil {
 					if strings.Contains(err.Error(), "is not a valid tag name") {
-						return models.ErrInvalidTagName{
+						return false, models.ErrInvalidTagName{
 							TagName: rel.TagName,
 						}
 					}
-					return err
+					return false, err
 				}
 			} else if err = gitRepo.CreateTag(rel.TagName, commit.ID.String()); err != nil {
 				if strings.Contains(err.Error(), "is not a valid tag name") {
-					return models.ErrInvalidTagName{
+					return false, models.ErrInvalidTagName{
 						TagName: rel.TagName,
 					}
 				}
-				return err
+				return false, err
 			}
+			created = true
 			rel.LowerTagName = strings.ToLower(rel.TagName)
 			// Prepare Notify
 			if err := rel.LoadAttributes(); err != nil {
 				log.Error("LoadAttributes: %v", err)
-				return err
+				return false, err
 			}
 			notification.NotifyPushCommits(
 				rel.Publisher, rel.Repo,
@@ -63,13 +66,13 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
 		}
 		commit, err := gitRepo.GetTagCommit(rel.TagName)
 		if err != nil {
-			return fmt.Errorf("GetTagCommit: %v", err)
+			return false, fmt.Errorf("GetTagCommit: %v", err)
 		}
 
 		rel.Sha1 = commit.ID.String()
 		rel.NumCommits, err = commit.CommitsCount()
 		if err != nil {
-			return fmt.Errorf("CommitsCount: %v", err)
+			return false, fmt.Errorf("CommitsCount: %v", err)
 		}
 
 		if rel.PublisherID <= 0 {
@@ -78,11 +81,10 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
 				rel.PublisherID = u.ID
 			}
 		}
-
 	} else {
 		rel.CreatedUnix = timeutil.TimeStampNow()
 	}
-	return nil
+	return created, nil
 }
 
 // CreateRelease creates a new release of repository.
@@ -96,7 +98,7 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs
 		}
 	}
 
-	if err = createTag(gitRepo, rel, msg); err != nil {
+	if _, err = createTag(gitRepo, rel, msg); err != nil {
 		return err
 	}
 
@@ -105,7 +107,7 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs
 		return err
 	}
 
-	if err = models.AddReleaseAttachments(rel.ID, attachmentUUIDs); err != nil {
+	if err = models.AddReleaseAttachments(models.DefaultDBContext(), rel.ID, attachmentUUIDs); err != nil {
 		return err
 	}
 
@@ -143,7 +145,7 @@ func CreateNewTag(doer *models.User, repo *models.Repository, commit, tagName, m
 		IsTag:        true,
 	}
 
-	if err = createTag(gitRepo, rel, msg); err != nil {
+	if _, err = createTag(gitRepo, rel, msg); err != nil {
 		return err
 	}
 
@@ -154,22 +156,97 @@ func CreateNewTag(doer *models.User, repo *models.Repository, commit, tagName, m
 	return err
 }
 
-// UpdateReleaseOrCreatReleaseFromTag updates information of a release or create release from tag.
-func UpdateReleaseOrCreatReleaseFromTag(doer *models.User, gitRepo *git.Repository, rel *models.Release, attachmentUUIDs []string, isCreate bool) (err error) {
-	if err = createTag(gitRepo, rel, ""); err != nil {
+// UpdateRelease updates information, attachments of a release and will create tag if it's not a draft and tag not exist.
+// addAttachmentUUIDs accept a slice of new created attachments' uuids which will be reassigned release_id as the created release
+// delAttachmentUUIDs accept a slice of attachments' uuids which will be deleted from the release
+// editAttachments accept a map of attachment uuid to new attachment name which will be updated with attachments.
+func UpdateRelease(doer *models.User, gitRepo *git.Repository, rel *models.Release,
+	addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string) (err error) {
+	if rel.ID == 0 {
+		return errors.New("UpdateRelease only accepts an exist release")
+	}
+	isCreated, err := createTag(gitRepo, rel, "")
+	if err != nil {
 		return err
 	}
 	rel.LowerTagName = strings.ToLower(rel.TagName)
 
-	if err = models.UpdateRelease(models.DefaultDBContext(), rel); err != nil {
+	ctx, commiter, err := models.TxDBContext()
+	if err != nil {
+		return err
+	}
+	defer commiter.Close()
+
+	if err = models.UpdateRelease(ctx, rel); err != nil {
 		return err
 	}
 
-	if err = models.AddReleaseAttachments(rel.ID, attachmentUUIDs); err != nil {
-		log.Error("AddReleaseAttachments: %v", err)
+	if err = models.AddReleaseAttachments(ctx, rel.ID, addAttachmentUUIDs); err != nil {
+		return fmt.Errorf("AddReleaseAttachments: %v", err)
+	}
+
+	var deletedUUIDsMap = make(map[string]bool)
+	if len(delAttachmentUUIDs) > 0 {
+		// Check attachments
+		attachments, err := models.GetAttachmentsByUUIDs(ctx, delAttachmentUUIDs)
+		if err != nil {
+			return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %v", delAttachmentUUIDs, err)
+		}
+		for _, attach := range attachments {
+			if attach.ReleaseID != rel.ID {
+				return errors.New("delete attachement of release permission denied")
+			}
+			deletedUUIDsMap[attach.UUID] = true
+		}
+
+		if _, err := models.DeleteAttachments(ctx, attachments, false); err != nil {
+			return fmt.Errorf("DeleteAttachments [uuids: %v]: %v", delAttachmentUUIDs, err)
+		}
+	}
+
+	if len(editAttachments) > 0 {
+		var updateAttachmentsList = make([]string, 0, len(editAttachments))
+		for k := range editAttachments {
+			updateAttachmentsList = append(updateAttachmentsList, k)
+		}
+		// Check attachments
+		attachments, err := models.GetAttachmentsByUUIDs(ctx, updateAttachmentsList)
+		if err != nil {
+			return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %v", updateAttachmentsList, err)
+		}
+		for _, attach := range attachments {
+			if attach.ReleaseID != rel.ID {
+				return errors.New("update attachement of release permission denied")
+			}
+		}
+
+		for uuid, newName := range editAttachments {
+			if !deletedUUIDsMap[uuid] {
+				if err = models.UpdateAttachmentByUUID(ctx, &models.Attachment{
+					UUID: uuid,
+					Name: newName,
+				}, "name"); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	if err = commiter.Commit(); err != nil {
+		return
+	}
+
+	for _, uuid := range delAttachmentUUIDs {
+		if err := storage.Attachments.Delete(models.AttachmentRelativePath(uuid)); err != nil {
+			// Even delete files failed, but the attachments has been removed from database, so we
+			// should not return error but only record the error on logs.
+			// users have to delete this attachments manually or we should have a
+			// synchronize between database attachment table and attachment storage
+			log.Error("delete attachment[uuid: %s] failed: %v", uuid, err)
+		}
 	}
 
-	if !isCreate {
+	if !isCreated {
 		notification.NotifyUpdateRelease(doer, rel)
 		return
 	}
diff --git a/services/release/release_test.go b/services/release/release_test.go
index deb0618832..102e3d7e0c 100644
--- a/services/release/release_test.go
+++ b/services/release/release_test.go
@@ -6,6 +6,7 @@ package release
 
 import (
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -90,7 +91,13 @@ func TestRelease_Create(t *testing.T) {
 		IsTag:        false,
 	}, nil, ""))
 
-	assert.NoError(t, CreateRelease(gitRepo, &models.Release{
+	attach, err := models.NewAttachment(&models.Attachment{
+		UploaderID: user.ID,
+		Name:       "test.txt",
+	}, []byte{}, strings.NewReader("testtest"))
+	assert.NoError(t, err)
+
+	var release = models.Release{
 		RepoID:       repo.ID,
 		PublisherID:  user.ID,
 		TagName:      "v0.1.5",
@@ -100,7 +107,8 @@ func TestRelease_Create(t *testing.T) {
 		IsDraft:      false,
 		IsPrerelease: false,
 		IsTag:        true,
-	}, nil, "test"))
+	}
+	assert.NoError(t, CreateRelease(gitRepo, &release, []string{attach.UUID}, "test"))
 }
 
 func TestRelease_Update(t *testing.T) {
@@ -131,7 +139,7 @@ func TestRelease_Update(t *testing.T) {
 	releaseCreatedUnix := release.CreatedUnix
 	time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
 	release.Note = "Changed note"
-	assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, false))
+	assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil))
 	release, err = models.GetReleaseByID(release.ID)
 	assert.NoError(t, err)
 	assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
@@ -153,7 +161,7 @@ func TestRelease_Update(t *testing.T) {
 	releaseCreatedUnix = release.CreatedUnix
 	time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
 	release.Title = "Changed title"
-	assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, false))
+	assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil))
 	release, err = models.GetReleaseByID(release.ID)
 	assert.NoError(t, err)
 	assert.Less(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
@@ -176,10 +184,64 @@ func TestRelease_Update(t *testing.T) {
 	time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
 	release.Title = "Changed title"
 	release.Note = "Changed note"
-	assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, false))
+	assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil))
 	release, err = models.GetReleaseByID(release.ID)
 	assert.NoError(t, err)
 	assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
+
+	// Test create release
+	release = &models.Release{
+		RepoID:       repo.ID,
+		PublisherID:  user.ID,
+		TagName:      "v1.1.2",
+		Target:       "master",
+		Title:        "v1.1.2 is released",
+		Note:         "v1.1.2 is released",
+		IsDraft:      true,
+		IsPrerelease: false,
+		IsTag:        false,
+	}
+	assert.NoError(t, CreateRelease(gitRepo, release, nil, ""))
+	assert.Greater(t, release.ID, int64(0))
+
+	release.IsDraft = false
+	tagName := release.TagName
+
+	assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil))
+	release, err = models.GetReleaseByID(release.ID)
+	assert.NoError(t, err)
+	assert.Equal(t, tagName, release.TagName)
+
+	// Add new attachments
+	attach, err := models.NewAttachment(&models.Attachment{
+		UploaderID: user.ID,
+		Name:       "test.txt",
+	}, []byte{}, strings.NewReader("testtest"))
+	assert.NoError(t, err)
+
+	assert.NoError(t, UpdateRelease(user, gitRepo, release, []string{attach.UUID}, nil, nil))
+	assert.NoError(t, models.GetReleaseAttachments(release))
+	assert.EqualValues(t, 1, len(release.Attachments))
+	assert.EqualValues(t, attach.UUID, release.Attachments[0].UUID)
+	assert.EqualValues(t, release.ID, release.Attachments[0].ReleaseID)
+	assert.EqualValues(t, attach.Name, release.Attachments[0].Name)
+
+	// update the attachment name
+	assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, map[string]string{
+		attach.UUID: "test2.txt",
+	}))
+	release.Attachments = nil
+	assert.NoError(t, models.GetReleaseAttachments(release))
+	assert.EqualValues(t, 1, len(release.Attachments))
+	assert.EqualValues(t, attach.UUID, release.Attachments[0].UUID)
+	assert.EqualValues(t, release.ID, release.Attachments[0].ReleaseID)
+	assert.EqualValues(t, "test2.txt", release.Attachments[0].Name)
+
+	// delete the attachment
+	assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, []string{attach.UUID}, nil))
+	release.Attachments = nil
+	assert.NoError(t, models.GetReleaseAttachments(release))
+	assert.EqualValues(t, 0, len(release.Attachments))
 }
 
 func TestRelease_createTag(t *testing.T) {
@@ -205,12 +267,14 @@ func TestRelease_createTag(t *testing.T) {
 		IsPrerelease: false,
 		IsTag:        false,
 	}
-	assert.NoError(t, createTag(gitRepo, release, ""))
+	_, err = createTag(gitRepo, release, "")
+	assert.NoError(t, err)
 	assert.NotEmpty(t, release.CreatedUnix)
 	releaseCreatedUnix := release.CreatedUnix
 	time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
 	release.Note = "Changed note"
-	assert.NoError(t, createTag(gitRepo, release, ""))
+	_, err = createTag(gitRepo, release, "")
+	assert.NoError(t, err)
 	assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
 
 	// Test a changed draft
@@ -225,11 +289,13 @@ func TestRelease_createTag(t *testing.T) {
 		IsPrerelease: false,
 		IsTag:        false,
 	}
-	assert.NoError(t, createTag(gitRepo, release, ""))
+	_, err = createTag(gitRepo, release, "")
+	assert.NoError(t, err)
 	releaseCreatedUnix = release.CreatedUnix
 	time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
 	release.Title = "Changed title"
-	assert.NoError(t, createTag(gitRepo, release, ""))
+	_, err = createTag(gitRepo, release, "")
+	assert.NoError(t, err)
 	assert.Less(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
 
 	// Test a changed pre-release
@@ -244,12 +310,14 @@ func TestRelease_createTag(t *testing.T) {
 		IsPrerelease: true,
 		IsTag:        false,
 	}
-	assert.NoError(t, createTag(gitRepo, release, ""))
+	_, err = createTag(gitRepo, release, "")
+	assert.NoError(t, err)
 	releaseCreatedUnix = release.CreatedUnix
 	time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
 	release.Title = "Changed title"
 	release.Note = "Changed note"
-	assert.NoError(t, createTag(gitRepo, release, ""))
+	_, err = createTag(gitRepo, release, "")
+	assert.NoError(t, err)
 	assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
 }
 
diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl
index 5271f650ee..72a69a756a 100644
--- a/templates/repo/release/list.tmpl
+++ b/templates/repo/release/list.tmpl
@@ -152,12 +152,14 @@
 										{{if .Attachments}}
 											{{range .Attachments}}
 												<li>
-													<span class="ui text right poping up" data-content="{{$.i18n.Tr "repo.release.download_count" (.DownloadCount | PrettyNumber)}}">
-														{{svg "octicon-info"}}
+													<span class="ui text middle aligned right">
+														<span class="ui text grey">{{.Size | FileSize}}</span>
+														<span class="poping up" data-content="{{$.i18n.Tr "repo.release.download_count" (.DownloadCount | PrettyNumber)}}">
+															{{svg "octicon-info"}}
+														</span>
 													</span>
 													<a target="_blank" rel="noopener noreferrer" href="{{.DownloadURL}}">
 														<strong><span class="ui image" title='{{.Name}}'>{{svg "octicon-package" 16 "mr-2"}}</span>{{.Name}}</strong>
-														<span class="ui text grey right">{{.Size | FileSize}}</span>
 													</a>
 												</li>
 											{{end}}
diff --git a/templates/repo/release/new.tmpl b/templates/repo/release/new.tmpl
index 485cc7138a..8489d89597 100644
--- a/templates/repo/release/new.tmpl
+++ b/templates/repo/release/new.tmpl
@@ -57,6 +57,23 @@
 						{{$.i18n.Tr "loading"}}
 					</div>
 				</div>
+				{{range .attachments}}
+					<div class="field" id="attachment-{{.ID}}">
+						<div class="ui right df ac wrap_remove">
+							<a class="ui mini compact red button remove-rel-attach" data-id="{{.ID}}" data-uuid="{{.UUID}}">
+								{{$.i18n.Tr "remove"}}
+							</a>
+						</div>
+						<div class="df ac">
+							<input name="attachment-edit-{{.UUID}}" class="mr-3 attachment_edit" required value="{{.Name}}"/>
+							<input name="attachment-del-{{.UUID}}" type="hidden" value="false"/>
+							<span class="ui text grey mr-3">{{.Size | FileSize}}</span>
+							<span class="poping up" data-content="{{$.i18n.Tr "repo.release.download_count" (.DownloadCount | PrettyNumber)}}">
+								{{svg "octicon-info"}}
+							</span>
+						</div>
+					</div>
+				{{end}}
 				{{if .IsAttachmentEnabled}}
 					<div class="field">
 						<div class="files"></div>
diff --git a/web_src/js/index.js b/web_src/js/index.js
index 81243c3916..de9b99d4ef 100644
--- a/web_src/js/index.js
+++ b/web_src/js/index.js
@@ -1255,6 +1255,15 @@ function initPullRequestMergeInstruction() {
   });
 }
 
+function initRelease() {
+  $(document).on('click', '.remove-rel-attach', function() {
+    const uuid = $(this).data('uuid');
+    const id = $(this).data('id');
+    $(`input[name='attachment-del-${uuid}']`).attr('value', true);
+    $(`#attachment-${id}`).hide();
+  });
+}
+
 function initPullRequestReview() {
   if (window.location.hash && window.location.hash.startsWith('#issuecomment-')) {
     const commentDiv = $(window.location.hash);
@@ -2748,6 +2757,7 @@ $(document).ready(async () => {
   initNotificationsTable();
   initPullRequestMergeInstruction();
   initReleaseEditor();
+  initRelease();
 
   const routes = {
     'div.user.settings': initUserSettings,
diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less
index 9f40c0c618..c49da7b395 100644
--- a/web_src/less/_repository.less
+++ b/web_src/less/_repository.less
@@ -1863,6 +1863,13 @@
           margin-bottom: 1em;
         }
       }
+
+      .wrap_remove {
+        height: 38px;
+      }
+      .attachment_edit {
+        width: 450px !important;
+      }
     }
   }