From f9ee426b641d613313e3aa0f4c1c6ce827d6f05f Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 6 Apr 2021 23:19:45 +0200 Subject: [PATCH] Make sure task attachments are properly attributed to link shares --- pkg/db/fixtures/task_attachments.yml | 5 +++++ pkg/models/task_attachment.go | 20 ++++++++++++++------ pkg/models/task_attachment_test.go | 7 ++++++- pkg/routes/routes.go | 2 +- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/pkg/db/fixtures/task_attachments.yml b/pkg/db/fixtures/task_attachments.yml index 9bbbcb0ed..c203d5b34 100644 --- a/pkg/db/fixtures/task_attachments.yml +++ b/pkg/db/fixtures/task_attachments.yml @@ -9,3 +9,8 @@ file_id: 9999 created_by_id: 1 created: 2018-12-01 15:13:12 +- id: 3 + task_id: 1 + file_id: 1 + created_by_id: -2 + created: 2018-12-01 15:13:12 diff --git a/pkg/models/task_attachment.go b/pkg/models/task_attachment.go index 457d16d2f..f82fe02bc 100644 --- a/pkg/models/task_attachment.go +++ b/pkg/models/task_attachment.go @@ -64,7 +64,17 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadCloser, realna // Add an entry to the db ta.FileID = file.ID - ta.CreatedByID = a.GetID() + + ta.CreatedBy, err = getUserOrLinkShareUser(s, a) + if err != nil { + // remove the uploaded file if adding it to the db fails + if err2 := file.Delete(); err2 != nil { + return err2 + } + return err + } + ta.CreatedByID = ta.CreatedBy.ID + _, err = s.Insert(ta) if err != nil { // remove the uploaded file if adding it to the db fails @@ -74,8 +84,6 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadCloser, realna return err } - ta.CreatedBy, _ = user.GetFromAuth(a) // Ignoring cases where the auth is not a user - return nil } @@ -145,19 +153,19 @@ func (ta *TaskAttachment) ReadAll(s *xorm.Session, a web.Auth, search string, pa return nil, 0, 0, err } - us := make(map[int64]*user.User) - err = s.In("id", userIDs).Find(&us) + users, err := getUsersOrLinkSharesFromIDs(s, userIDs) if err != nil { return nil, 0, 0, err } for _, r := range attachments { + r.CreatedBy = users[r.CreatedByID] + // If the actual file does not exist, don't try to load it as that would fail with nil panic if _, exists := fs[r.FileID]; !exists { continue } r.File = fs[r.FileID] - r.CreatedBy = us[r.CreatedByID] } numberOfTotalItems, err = s. diff --git a/pkg/models/task_attachment_test.go b/pkg/models/task_attachment_test.go index 7d6dbad56..47650deea 100644 --- a/pkg/models/task_attachment_test.go +++ b/pkg/models/task_attachment_test.go @@ -146,8 +146,13 @@ func TestTaskAttachment_ReadAll(t *testing.T) { as, _, _, err := ta.ReadAll(s, &user.User{ID: 1}, "", 0, 50) attachments, _ := as.([]*TaskAttachment) assert.NoError(t, err) - assert.Len(t, attachments, 2) + assert.Len(t, attachments, 3) assert.Equal(t, "test", attachments[0].File.Name) + for _, a := range attachments { + assert.NotNil(t, a.CreatedBy) + } + assert.Equal(t, int64(-2), attachments[2].CreatedByID) + assert.Equal(t, int64(-2), attachments[2].CreatedBy.ID) } func TestTaskAttachment_Delete(t *testing.T) { diff --git a/pkg/routes/routes.go b/pkg/routes/routes.go index 7232fe622..6bd375181 100644 --- a/pkg/routes/routes.go +++ b/pkg/routes/routes.go @@ -6,7 +6,7 @@ // the Free Software Foundation, either version 3 of the License, or // (at your option) any later version. // -// This program is distributed in the hope that it will be useful, +// This program is distribuTaskAttted in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU Affero General Public Licensee for more details.