diff --git a/services/release/release.go b/services/release/release.go index 3cd049a27e..e041486e3e 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -267,7 +267,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo addAttachmentUUIDs := make(container.Set[string]) delAttachmentUUIDs := make(container.Set[string]) updateAttachmentUUIDs := make(container.Set[string]) - updateAttachments := make(container.Set[*AttachmentChange]) + updateAttachments := map[string]*AttachmentChange{} for _, attachmentChange := range attachmentChanges { switch attachmentChange.Action { @@ -305,7 +305,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo delAttachmentUUIDs.Add(attachmentChange.UUID) case "update": updateAttachmentUUIDs.Add(attachmentChange.UUID) - updateAttachments.Add(attachmentChange) + updateAttachments[attachmentChange.UUID] = attachmentChange default: if attachmentChange.Action == "" { return fmt.Errorf("missing attachment action") @@ -318,16 +318,12 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo return fmt.Errorf("AddReleaseAttachments: %w", err) } - deletedUUIDs := make(container.Set[string]) if len(delAttachmentUUIDs) > 0 { // Check delAttachments delAttachments, err := repo_model.FindRepoAttachmentsByUUID(ctx, rel.RepoID, delAttachmentUUIDs.Values(), repo_model.FindAttachmentOptions{ReleaseID: rel.ID}) if err != nil { return fmt.Errorf("FindRepoAttachmentsByUUID[uuids=%q,repoID=%d,releaseID=%d]: %w", delAttachmentUUIDs.Values(), rel.RepoID, rel.ID, err) } - for _, attach := range delAttachments { - deletedUUIDs.Add(attach.UUID) - } if _, err := repo_model.DeleteAttachments(ctx, delAttachments, true); err != nil { return fmt.Errorf("DeleteAttachments [uuids: %v]: %w", delAttachmentUUIDs, err) @@ -341,16 +337,12 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo return fmt.Errorf("FindRepoAttachmentsByUUID[uuids=%q,repoID=%d,releaseID=%d]: %w", updateAttachmentUUIDs.Values(), rel.RepoID, rel.ID, err) } - if len(attachments) != len(updateAttachments) { - return util.SilentWrap{ - Message: "update attachment of release permission denied", - Err: util.ErrPermissionDenied, + for _, attachment := range attachments { + attachmentChange, ok := updateAttachments[attachment.UUID] + if !ok { + continue } - } - } - for attachmentChange := range updateAttachments { - if !deletedUUIDs.Contains(attachmentChange.UUID) { if err = repo_model.UpdateAttachmentByUUID(ctx, &repo_model.Attachment{ UUID: attachmentChange.UUID, Name: attachmentChange.Name, diff --git a/services/release/release_test.go b/services/release/release_test.go index 40e5e51bca..5dde1e36eb 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -14,7 +14,6 @@ import ( user_model "forgejo.org/models/user" "forgejo.org/modules/git" "forgejo.org/modules/gitrepo" - "forgejo.org/modules/util" "forgejo.org/services/attachment" _ "forgejo.org/models/actions" @@ -406,13 +405,48 @@ func TestRelease_Update(t *testing.T) { require.NoError(t, repo_model.GetReleaseAttachments(t.Context(), release)) assert.Empty(t, release.Attachments) - require.ErrorIs(t, UpdateRelease(t.Context(), user, gitRepo, release, false, []*AttachmentChange{ + require.NoError(t, UpdateRelease(t.Context(), user, gitRepo, release, false, []*AttachmentChange{ { Action: "update", Name: "test2.txt", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a13", }, - }), util.ErrPermissionDenied) + })) + require.NoError(t, repo_model.GetReleaseAttachments(t.Context(), release)) + assert.Empty(t, release.Attachments) + }) + + t.Run("Delete and edit", func(t *testing.T) { + // Add a attachment. + attach, err := attachment.NewAttachment(t.Context(), &repo_model.Attachment{ + RepoID: repo.ID, + UploaderID: user.ID, + Name: "test.txt", + }, strings.NewReader("beep"), 4) + require.NoError(t, err) + + require.NoError(t, UpdateRelease(t.Context(), user, gitRepo, release, false, []*AttachmentChange{ + { + Action: "add", + Type: "attachment", + UUID: attach.UUID, + }, + })) + require.NoError(t, repo_model.GetReleaseAttachments(t.Context(), release)) + assert.Len(t, release.Attachments, 1) + + // Delete and edit at the same time, this is identical how the browser would delete a attachment. + require.NoError(t, UpdateRelease(t.Context(), user, gitRepo, release, false, []*AttachmentChange{ + { + Action: "update", + Name: "test.txt", + UUID: attach.UUID, + }, + { + Action: "delete", + UUID: attach.UUID, + }, + })) require.NoError(t, repo_model.GetReleaseAttachments(t.Context(), release)) assert.Empty(t, release.Attachments) })