From 9ca9540eba6e64cf5d8f5246f6ed31d9bdd843bc Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Tue, 6 Apr 2021 12:08:19 +0200
Subject: [PATCH] Make sure to return authors for comments created by link
 shares

---
 pkg/db/fixtures/task_comments.yml     |  6 ++++++
 pkg/integrations/task_comment_test.go |  6 ++++++
 pkg/models/link_sharing.go            |  7 +++++++
 pkg/models/task_comments.go           | 14 ++++++-------
 pkg/models/task_comments_test.go      | 14 +++++++++++++
 pkg/models/users.go                   | 30 +++++++++++++++++++++++++++
 6 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/pkg/db/fixtures/task_comments.yml b/pkg/db/fixtures/task_comments.yml
index 16ce99a73..83163ed74 100644
--- a/pkg/db/fixtures/task_comments.yml
+++ b/pkg/db/fixtures/task_comments.yml
@@ -94,3 +94,9 @@
   task_id: 36
   created: 2020-02-19 18:07:06
   updated: 2020-02-19 18:07:06
+- id: 17
+  comment: comment 17
+  author_id: -2
+  task_id: 35
+  created: 2020-02-19 18:07:06
+  updated: 2020-02-19 18:07:06
diff --git a/pkg/integrations/task_comment_test.go b/pkg/integrations/task_comment_test.go
index 336ac6547..2fa58b2ae 100644
--- a/pkg/integrations/task_comment_test.go
+++ b/pkg/integrations/task_comment_test.go
@@ -17,6 +17,7 @@
 package integrations
 
 import (
+	"code.vikunja.io/api/pkg/db"
 	"testing"
 
 	"code.vikunja.io/api/pkg/models"
@@ -298,6 +299,11 @@ func TestTaskComments(t *testing.T) {
 				rec, err := testHandlerLinkShareWrite.testCreateWithLinkShare(nil, map[string]string{"task": "13"}, `{"comment":"Lorem Ipsum"}`)
 				assert.NoError(t, err)
 				assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`)
+				db.AssertExists(t, "task_comments", map[string]interface{}{
+					"task_id":   13,
+					"comment":   "Lorem Ipsum",
+					"author_id": -2,
+				}, false)
 			})
 		})
 	})
diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go
index f4d0e2552..e8835e2d6 100644
--- a/pkg/models/link_sharing.go
+++ b/pkg/models/link_sharing.go
@@ -280,3 +280,10 @@ func GetLinkShareByID(s *xorm.Session, id int64) (share *LinkSharing, err error)
 	}
 	return
 }
+
+// GetLinkSharesByIDs returns all link shares from a slice of ids
+func GetLinkSharesByIDs(s *xorm.Session, ids []int64) (shares map[int64]*LinkSharing, err error) {
+	shares = make(map[int64]*LinkSharing)
+	err = s.In("id", ids).Find(&shares)
+	return
+}
diff --git a/pkg/models/task_comments.go b/pkg/models/task_comments.go
index 4e9003335..d213ef884 100644
--- a/pkg/models/task_comments.go
+++ b/pkg/models/task_comments.go
@@ -213,14 +213,12 @@ func (tc *TaskComment) ReadAll(s *xorm.Session, auth web.Auth, search string, pa
 		return
 	}
 
-	// Get all authors
-	authors := make(map[int64]*user.User)
-	err = s.
-		Select("users.*").
-		Table("task_comments").
-		Where("task_id = ? AND comment like ?", tc.TaskID, "%"+search+"%").
-		Join("INNER", "users", "users.id = task_comments.author_id").
-		Find(&authors)
+	var authorIDs []int64
+	for _, comment := range comments {
+		authorIDs = append(authorIDs, comment.AuthorID)
+	}
+
+	authors, err := getUsersOrLinkSharesFromIDs(s, authorIDs)
 	if err != nil {
 		return
 	}
diff --git a/pkg/models/task_comments_test.go b/pkg/models/task_comments_test.go
index 3ad3abf8d..ecc16ae07 100644
--- a/pkg/models/task_comments_test.go
+++ b/pkg/models/task_comments_test.go
@@ -184,4 +184,18 @@ func TestTaskComment_ReadAll(t *testing.T) {
 		assert.Error(t, err)
 		assert.True(t, IsErrGenericForbidden(err))
 	})
+	t.Run("comment from link share", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		tc := &TaskComment{TaskID: 35}
+		u := &user.User{ID: 1}
+		result, _, _, err := tc.ReadAll(s, u, "", 0, -1)
+		comments := result.([]*TaskComment)
+		assert.NoError(t, err)
+		assert.Len(t, comments, 2)
+		assert.Equal(t, int64(-2), comments[1].AuthorID)
+		assert.NotNil(t, comments[1].Author)
+	})
 }
diff --git a/pkg/models/users.go b/pkg/models/users.go
index 53e3f72ca..8bb1f9a59 100644
--- a/pkg/models/users.go
+++ b/pkg/models/users.go
@@ -40,3 +40,33 @@ func getUserOrLinkShareUser(s *xorm.Session, a web.Auth) (uu *user.User, err err
 
 	return
 }
+
+// Returns all users or pseudo link shares from a slice of ids. ids < 0 are considered to be a link share in that case.
+func getUsersOrLinkSharesFromIDs(s *xorm.Session, ids []int64) (users map[int64]*user.User, err error) {
+	var userIDs []int64
+	var linkShareIDs []int64
+	for _, id := range ids {
+		if id < 0 {
+			linkShareIDs = append(linkShareIDs, id*-1)
+			continue
+		}
+
+		userIDs = append(userIDs, id)
+	}
+
+	users, err = user.GetUsersByIDs(s, userIDs)
+	if err != nil {
+		return
+	}
+
+	shares, err := GetLinkSharesByIDs(s, linkShareIDs)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, share := range shares {
+		users[share.ID*-1] = share.toUser()
+	}
+
+	return
+}