feat(comments): treat quoted comment authors as implicit mentions
A comment whose body contains <blockquote data-comment-id="…"> nodes now triggers the same task-comment mention notification for the quoted comments' authors, respecting CanRead, subscription, and existing dedup. Self-quotes, wrong-task quotes, and malformed ids are silently skipped.
This commit is contained in:
kolaente
kolaente
parent
a1f81524ab
commit
6fc36cb700
|
|
@ -0,0 +1 @@
|
|||
[]
|
||||
|
|
@ -0,0 +1,124 @@
|
|||
// Vikunja is a to-do list application to facilitate your life.
|
||||
// Copyright 2018-present Vikunja and contributors. All rights reserved.
|
||||
//
|
||||
// This program is free software: you can redistribute it and/or modify
|
||||
// it under the terms of the GNU Affero General Public License as published by
|
||||
// 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,
|
||||
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
// GNU Affero General Public License for more details.
|
||||
//
|
||||
// You should have received a copy of the GNU Affero General Public License
|
||||
// along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
|
||||
package models
|
||||
|
||||
import (
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"code.vikunja.io/api/pkg/user"
|
||||
|
||||
"golang.org/x/net/html"
|
||||
"xorm.io/xorm"
|
||||
)
|
||||
|
||||
// extractQuotedCommentIDs parses HTML and returns the set of positive integer
|
||||
// comment ids referenced by <blockquote data-comment-id="…"> nodes anywhere in
|
||||
// the document. Malformed values and zero/negative ids are silently skipped.
|
||||
func extractQuotedCommentIDs(htmlText string) []int64 {
|
||||
if htmlText == "" {
|
||||
return nil
|
||||
}
|
||||
|
||||
doc, err := html.Parse(strings.NewReader(htmlText))
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
ids := []int64{}
|
||||
seen := make(map[int64]bool)
|
||||
|
||||
var traverse func(*html.Node)
|
||||
traverse = func(n *html.Node) {
|
||||
if n.Type == html.ElementNode && n.Data == "blockquote" {
|
||||
for _, attr := range n.Attr {
|
||||
if attr.Key != "data-comment-id" {
|
||||
continue
|
||||
}
|
||||
id, err := strconv.ParseInt(strings.TrimSpace(attr.Val), 10, 64)
|
||||
if err == nil && id > 0 && !seen[id] {
|
||||
ids = append(ids, id)
|
||||
seen[id] = true
|
||||
}
|
||||
break
|
||||
}
|
||||
}
|
||||
for c := n.FirstChild; c != nil; c = c.NextSibling {
|
||||
traverse(c)
|
||||
}
|
||||
}
|
||||
traverse(doc)
|
||||
return ids
|
||||
}
|
||||
|
||||
// findQuotedCommentAuthors resolves the authors of comments referenced via
|
||||
// <blockquote data-comment-id="…"> within text, restricted to comments on the
|
||||
// given task and excluding the doer themselves. Missing or wrong-task
|
||||
// references are silently skipped. Link-share authors (negative ids) are
|
||||
// also skipped since they cannot receive notifications.
|
||||
func findQuotedCommentAuthors(s *xorm.Session, taskID, doerID int64, text string) (map[int64]*user.User, error) {
|
||||
ids := extractQuotedCommentIDs(text)
|
||||
if len(ids) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
rows := []*TaskComment{}
|
||||
err := s.
|
||||
In("id", ids).
|
||||
And("task_id = ?", taskID).
|
||||
Cols("id", "author_id", "task_id").
|
||||
Find(&rows)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if len(rows) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
dedup := map[int64]bool{}
|
||||
wanted := []int64{}
|
||||
for _, r := range rows {
|
||||
id := r.AuthorID
|
||||
if id == doerID {
|
||||
continue
|
||||
}
|
||||
if id <= 0 {
|
||||
continue
|
||||
}
|
||||
if dedup[id] {
|
||||
continue
|
||||
}
|
||||
dedup[id] = true
|
||||
wanted = append(wanted, id)
|
||||
}
|
||||
if len(wanted) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
list := []*user.User{}
|
||||
err = s.In("id", wanted).Find(&list)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
users := make(map[int64]*user.User, len(list))
|
||||
for _, u := range list {
|
||||
users[u.ID] = u
|
||||
}
|
||||
return users, nil
|
||||
}
|
||||
|
|
@ -0,0 +1,344 @@
|
|||
// Vikunja is a to-do list application to facilitate your life.
|
||||
// Copyright 2018-present Vikunja and contributors. All rights reserved.
|
||||
//
|
||||
// This program is free software: you can redistribute it and/or modify
|
||||
// it under the terms of the GNU Affero General Public License as published by
|
||||
// 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,
|
||||
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
// GNU Affero General Public License for more details.
|
||||
//
|
||||
// You should have received a copy of the GNU Affero General Public License
|
||||
// along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
|
||||
package models
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"code.vikunja.io/api/pkg/db"
|
||||
"code.vikunja.io/api/pkg/events"
|
||||
"code.vikunja.io/api/pkg/user"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"xorm.io/builder"
|
||||
)
|
||||
|
||||
func TestExtractQuotedCommentIDs(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
in string
|
||||
out []int64
|
||||
}{
|
||||
{"empty", "", nil},
|
||||
{"no blockquote", `<p>hello</p>`, []int64{}},
|
||||
{"plain blockquote without attr", `<blockquote>hi</blockquote>`, []int64{}},
|
||||
{"single attributed quote", `<blockquote data-comment-id="42">hi</blockquote>`, []int64{42}},
|
||||
{
|
||||
"nested inside paragraph",
|
||||
`<p><blockquote data-comment-id="7">hi</blockquote></p>`,
|
||||
[]int64{7},
|
||||
},
|
||||
{
|
||||
"two quotes - deduped order preserved",
|
||||
`<blockquote data-comment-id="3">a</blockquote><blockquote data-comment-id="5">b</blockquote><blockquote data-comment-id="3">a again</blockquote>`,
|
||||
[]int64{3, 5},
|
||||
},
|
||||
{"malformed - non-numeric", `<blockquote data-comment-id="abc">hi</blockquote>`, []int64{}},
|
||||
{"malformed - negative", `<blockquote data-comment-id="-1">hi</blockquote>`, []int64{}},
|
||||
{"malformed - zero", `<blockquote data-comment-id="0">hi</blockquote>`, []int64{}},
|
||||
{"malformed - empty", `<blockquote data-comment-id="">hi</blockquote>`, []int64{}},
|
||||
{
|
||||
"nested blockquote inside blockquote",
|
||||
`<blockquote data-comment-id="9"><blockquote data-comment-id="8">inner</blockquote></blockquote>`,
|
||||
[]int64{9, 8},
|
||||
},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
got := extractQuotedCommentIDs(c.in)
|
||||
if c.out == nil {
|
||||
assert.Nil(t, got)
|
||||
return
|
||||
}
|
||||
assert.Equal(t, c.out, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// notifQuery builds a where clause matching a TaskCommentNotification for a
|
||||
// given subject + recipient.
|
||||
func notifQuery(subjectID, userID int64) builder.Cond {
|
||||
return builder.And(
|
||||
builder.Eq{"subject_id": subjectID},
|
||||
builder.Eq{"notifiable_id": userID},
|
||||
builder.Eq{"name": (&TaskCommentNotification{}).Name()},
|
||||
)
|
||||
}
|
||||
|
||||
func TestTaskComment_CommentReplies_Notifications(t *testing.T) {
|
||||
doer := &user.User{ID: 1}
|
||||
|
||||
// task 32 is owned by user 1 (the doer) on project 3.
|
||||
// user 2 has access to project 3 (this is exercised by the existing
|
||||
// "should send notifications for comment mentions" test).
|
||||
|
||||
t.Run("blockquote pointing at a same-task comment authored by another user notifies that user once", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
// existing comment by user 2 on task 32
|
||||
parent := &TaskComment{Comment: "original", TaskID: 32, AuthorID: 2}
|
||||
_, err := s.Insert(parent)
|
||||
require.NoError(t, err)
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: fmt.Sprintf(`<blockquote data-comment-id="%d">original</blockquote><p>thanks!</p>`, parent.ID),
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertCount(t, "notifications", notifQuery(tc.ID, 2), 1)
|
||||
})
|
||||
|
||||
t.Run("blockquote and @mention referring to the same user still result in exactly one notification", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
parent := &TaskComment{Comment: "original", TaskID: 32, AuthorID: 2}
|
||||
_, err := s.Insert(parent)
|
||||
require.NoError(t, err)
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: fmt.Sprintf(
|
||||
`<p><mention-user data-id="user2">@user2</mention-user></p><blockquote data-comment-id="%d">original</blockquote>`,
|
||||
parent.ID,
|
||||
),
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertCount(t, "notifications", notifQuery(tc.ID, 2), 1)
|
||||
})
|
||||
|
||||
t.Run("two blockquotes pointing at comments by two different users each notify their author once", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
parentB := &TaskComment{Comment: "by B", TaskID: 32, AuthorID: 2}
|
||||
_, err := s.Insert(parentB)
|
||||
require.NoError(t, err)
|
||||
parentC := &TaskComment{Comment: "by C", TaskID: 32, AuthorID: 3}
|
||||
_, err = s.Insert(parentC)
|
||||
require.NoError(t, err)
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: fmt.Sprintf(
|
||||
`<blockquote data-comment-id="%d">by B</blockquote><blockquote data-comment-id="%d">by C</blockquote>`,
|
||||
parentB.ID, parentC.ID,
|
||||
),
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertCount(t, "notifications", notifQuery(tc.ID, 2), 1)
|
||||
db.AssertCount(t, "notifications", notifQuery(tc.ID, 3), 1)
|
||||
})
|
||||
|
||||
t.Run("blockquote pointing at a comment on a different task contributes nothing", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
// user 2 authored a comment on a different task (task 1, owned by user 1).
|
||||
offTask := &TaskComment{Comment: "elsewhere", TaskID: 1, AuthorID: 2}
|
||||
_, err := s.Insert(offTask)
|
||||
require.NoError(t, err)
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: fmt.Sprintf(`<blockquote data-comment-id="%d">elsewhere</blockquote>`, offTask.ID),
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertMissing(t, "notifications", map[string]interface{}{
|
||||
"subject_id": tc.ID,
|
||||
"notifiable_id": 2,
|
||||
"name": (&TaskCommentNotification{}).Name(),
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("blockquote pointing at a missing comment is silently ignored", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: `<blockquote data-comment-id="99999">missing</blockquote>`,
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertMissing(t, "notifications", map[string]interface{}{
|
||||
"subject_id": tc.ID,
|
||||
"name": (&TaskCommentNotification{}).Name(),
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("blockquote pointing at the replier's own comment does not self-notify", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
mine := &TaskComment{Comment: "by me", TaskID: 32, AuthorID: 1}
|
||||
_, err := s.Insert(mine)
|
||||
require.NoError(t, err)
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: fmt.Sprintf(`<blockquote data-comment-id="%d">by me</blockquote>`, mine.ID),
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertMissing(t, "notifications", map[string]interface{}{
|
||||
"subject_id": tc.ID,
|
||||
"notifiable_id": 1,
|
||||
"name": (&TaskCommentNotification{}).Name(),
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("blockquote with non-integer data-comment-id triggers no DB lookup and no notification", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: `<blockquote data-comment-id="abc">malformed</blockquote>`,
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertMissing(t, "notifications", map[string]interface{}{
|
||||
"subject_id": tc.ID,
|
||||
"name": (&TaskCommentNotification{}).Name(),
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("blockquote nested deeper in the document is still counted", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
parent := &TaskComment{Comment: "original", TaskID: 32, AuthorID: 2}
|
||||
_, err := s.Insert(parent)
|
||||
require.NoError(t, err)
|
||||
|
||||
task, err := GetTaskByIDSimple(s, 32)
|
||||
require.NoError(t, err)
|
||||
|
||||
tc := &TaskComment{
|
||||
Comment: fmt.Sprintf(
|
||||
`<div><section><blockquote data-comment-id="%d">deep</blockquote></section></div>`,
|
||||
parent.ID,
|
||||
),
|
||||
TaskID: 32,
|
||||
}
|
||||
err = tc.Create(s, doer)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.Commit())
|
||||
|
||||
events.TestListener(t, &TaskCommentCreatedEvent{
|
||||
Task: &task,
|
||||
Doer: doer,
|
||||
Comment: tc,
|
||||
}, &SendTaskCommentNotification{})
|
||||
|
||||
db.AssertCount(t, "notifications", notifQuery(tc.ID, 2), 1)
|
||||
})
|
||||
}
|
||||
|
|
@ -209,6 +209,45 @@ func (s *SendTaskCommentNotification) Handle(msg *message.Message) (err error) {
|
|||
return err
|
||||
}
|
||||
|
||||
// Authors of comments quoted via <blockquote data-comment-id="…"> are
|
||||
// treated as implicit mentions, sharing the same notification, dedup,
|
||||
// permission and subscription logic.
|
||||
quotedAuthors, err := findQuotedCommentAuthors(sess, event.Task.ID, event.Doer.ID, event.Comment.Comment)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for _, u := range quotedAuthors {
|
||||
if _, has := mentionedUsers[u.ID]; has {
|
||||
continue
|
||||
}
|
||||
|
||||
can, _, err := event.Task.CanRead(sess, u)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !can {
|
||||
continue
|
||||
}
|
||||
|
||||
dbn, err := notifications.GetNotificationsForNameAndUser(sess, u.ID, n.Name(), n.SubjectID())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if len(dbn) > 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
err = notifications.Notify(u, n, sess)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if mentionedUsers == nil {
|
||||
mentionedUsers = make(map[int64]*user.User)
|
||||
}
|
||||
mentionedUsers[u.ID] = u
|
||||
}
|
||||
|
||||
subscribers, err := GetSubscriptionsForEntity(sess, SubscriptionEntityTask, event.Task.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -78,6 +78,7 @@ func SetupTests() {
|
|||
"webhooks",
|
||||
"totp",
|
||||
"oauth_codes",
|
||||
"notifications",
|
||||
)
|
||||
if err != nil {
|
||||
log.Fatal(err)
|
||||
|
|
|
|||
Loading…
Reference in New Issue