From 2cd2e64a834f857c8c1d78e86d6ddd68c270998a Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Mon, 5 Apr 2021 18:13:45 +0200
Subject: [PATCH] Make comments actually work for link shares

---
 pkg/integrations/task_comment_test.go | 19 ++++++++++++
 pkg/models/link_sharing.go            | 13 +++++++++
 pkg/models/task_comments.go           | 20 ++++---------
 pkg/models/users.go                   | 42 +++++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 14 deletions(-)
 create mode 100644 pkg/models/users.go

diff --git a/pkg/integrations/task_comment_test.go b/pkg/integrations/task_comment_test.go
index 59e3a92d1..336ac6547 100644
--- a/pkg/integrations/task_comment_test.go
+++ b/pkg/integrations/task_comment_test.go
@@ -33,6 +33,20 @@ func TestTaskComments(t *testing.T) {
 		},
 		t: t,
 	}
+	testHandlerLinkShareWrite := webHandlerTest{
+		linkShare: &models.LinkSharing{
+			ID:          2,
+			Hash:        "test2",
+			ListID:      2,
+			Right:       models.RightWrite,
+			SharingType: models.SharingTypeWithoutPassword,
+			SharedByID:  1,
+		},
+		strFunc: func() handler.CObject {
+			return &models.TaskComment{}
+		},
+		t: t,
+	}
 	// Only run specific nested tests:
 	// ^TestTaskComments$/^Update$/^Update_task_items$/^Removing_Assignees_null$
 	t.Run("Update", func(t *testing.T) {
@@ -280,6 +294,11 @@ func TestTaskComments(t *testing.T) {
 				assert.NoError(t, err)
 				assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`)
 			})
+			t.Run("Link Share", func(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"`)
+			})
 		})
 	})
 }
diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go
index 4540fe76a..f4d0e2552 100644
--- a/pkg/models/link_sharing.go
+++ b/pkg/models/link_sharing.go
@@ -267,3 +267,16 @@ func GetListByShareHash(s *xorm.Session, hash string) (list *List, err error) {
 	list, err = GetListSimpleByID(s, share.ListID)
 	return
 }
+
+// GetLinkShareByID returns a link share by its id.
+func GetLinkShareByID(s *xorm.Session, id int64) (share *LinkSharing, err error) {
+	share = &LinkSharing{}
+	has, err := s.Where("id = ?", id).Get(share)
+	if err != nil {
+		return
+	}
+	if !has {
+		return share, ErrListShareDoesNotExist{ID: id}
+	}
+	return
+}
diff --git a/pkg/models/task_comments.go b/pkg/models/task_comments.go
index d3ce74cc6..4e9003335 100644
--- a/pkg/models/task_comments.go
+++ b/pkg/models/task_comments.go
@@ -67,30 +67,22 @@ func (tc *TaskComment) Create(s *xorm.Session, a web.Auth) (err error) {
 		return err
 	}
 
-	tc.AuthorID = a.GetID()
-
-	if share, is := a.(*LinkSharing); is {
-		tc.AuthorID = share.getUserID()
+	tc.Author, err = getUserOrLinkShareUser(s, a)
+	if err != nil {
+		return err
 	}
+	tc.AuthorID = tc.Author.ID
 
 	_, err = s.Insert(tc)
 	if err != nil {
 		return
 	}
 
-	doer, _ := user.GetFromAuth(a)
-	err = events.Dispatch(&TaskCommentCreatedEvent{
+	return events.Dispatch(&TaskCommentCreatedEvent{
 		Task:    &task,
 		Comment: tc,
-		Doer:    doer,
+		Doer:    tc.Author,
 	})
-	if err != nil {
-		return err
-	}
-
-	// TODO: Add a wrapper in models which returns either a user or a share disguised as a share
-	tc.Author, err = user.GetUserByID(s, tc.AuthorID)
-	return
 }
 
 // Delete removes a task comment
diff --git a/pkg/models/users.go b/pkg/models/users.go
new file mode 100644
index 000000000..53e3f72ca
--- /dev/null
+++ b/pkg/models/users.go
@@ -0,0 +1,42 @@
+// Copyright 2021 Vikunja and contriubtors. All rights reserved.
+//
+// This file is part of Vikunja.
+//
+// Vikunja is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Vikunja is distributed 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 General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Vikunja.  If not, see <https://www.gnu.org/licenses/>.
+
+package models
+
+import (
+	"code.vikunja.io/api/pkg/user"
+	"code.vikunja.io/web"
+	"xorm.io/xorm"
+)
+
+// Returns either a user or a link share disguised as a user.
+func getUserOrLinkShareUser(s *xorm.Session, a web.Auth) (uu *user.User, err error) {
+	if u, is := a.(*user.User); is {
+		uu, err = user.GetUserByID(s, u.ID)
+		return
+	}
+
+	if ls, is := a.(*LinkSharing); is {
+		l, err := GetLinkShareByID(s, ls.ID)
+		if err != nil {
+			return nil, err
+		}
+		return l.toUser(), nil
+	}
+
+	return
+}