From 6fc36cb7001af97969c3e88d36f1c858e19decd5 Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 19 May 2026 19:15:23 +0200 Subject: [PATCH] feat(comments): treat quoted comment authors as implicit mentions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A comment whose body contains
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. --- pkg/db/fixtures/notifications.yml | 1 + pkg/models/comment_quotes.go | 124 +++++++++++ pkg/models/comment_quotes_test.go | 344 ++++++++++++++++++++++++++++++ pkg/models/listeners.go | 39 ++++ pkg/models/setup_tests.go | 1 + 5 files changed, 509 insertions(+) create mode 100644 pkg/db/fixtures/notifications.yml create mode 100644 pkg/models/comment_quotes.go create mode 100644 pkg/models/comment_quotes_test.go diff --git a/pkg/db/fixtures/notifications.yml b/pkg/db/fixtures/notifications.yml new file mode 100644 index 000000000..fe51488c7 --- /dev/null +++ b/pkg/db/fixtures/notifications.yml @@ -0,0 +1 @@ +[] diff --git a/pkg/models/comment_quotes.go b/pkg/models/comment_quotes.go new file mode 100644 index 000000000..808895b0c --- /dev/null +++ b/pkg/models/comment_quotes.go @@ -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 . + +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
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 +//
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 +} diff --git a/pkg/models/comment_quotes_test.go b/pkg/models/comment_quotes_test.go new file mode 100644 index 000000000..e45068e61 --- /dev/null +++ b/pkg/models/comment_quotes_test.go @@ -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 . + +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", `

hello

`, []int64{}}, + {"plain blockquote without attr", `
hi
`, []int64{}}, + {"single attributed quote", `
hi
`, []int64{42}}, + { + "nested inside paragraph", + `

hi

`, + []int64{7}, + }, + { + "two quotes - deduped order preserved", + `
a
b
a again
`, + []int64{3, 5}, + }, + {"malformed - non-numeric", `
hi
`, []int64{}}, + {"malformed - negative", `
hi
`, []int64{}}, + {"malformed - zero", `
hi
`, []int64{}}, + {"malformed - empty", `
hi
`, []int64{}}, + { + "nested blockquote inside blockquote", + `
inner
`, + []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(`
original

thanks!

`, 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( + `

@user2

original
`, + 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( + `
by B
by C
`, + 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(`
elsewhere
`, 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: `
missing
`, + 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(`
by me
`, 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: `
malformed
`, + 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( + `
deep
`, + 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) + }) +} diff --git a/pkg/models/listeners.go b/pkg/models/listeners.go index 86707c94e..b0a580c21 100644 --- a/pkg/models/listeners.go +++ b/pkg/models/listeners.go @@ -209,6 +209,45 @@ func (s *SendTaskCommentNotification) Handle(msg *message.Message) (err error) { return err } + // Authors of comments quoted via
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 diff --git a/pkg/models/setup_tests.go b/pkg/models/setup_tests.go index 90b31927a..66948fc84 100644 --- a/pkg/models/setup_tests.go +++ b/pkg/models/setup_tests.go @@ -78,6 +78,7 @@ func SetupTests() { "webhooks", "totp", "oauth_codes", + "notifications", ) if err != nil { log.Fatal(err)