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

* 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

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

Author: kdumontnu

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,14 +230,23 @@ 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
 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 == "" {

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)

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

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
 	}
 

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
 	}