Commit c2923bffd4
Verified · cmc ci/build: success ci/test: success
.gitbay/wiki/Users.org +6 −1
| @@ -517,7 +517,12 @@ handled. | ||
| 517 | 517 | |
| 518 | 518 | When the instance has SMTP configured, activity mails you: someone |
| 519 | 519 | opens an issue or MR on your repository, comments where you are a |
| 520 | participant (author, commenter, reviewer), reviews, closes, or merges. | |
| 520 | participant (author, commenter, reviewer, or mentioned), reviews, | |
| 521 | closes, or merges. Writing =@name= in an issue, merge request or | |
| 522 | comment files "mentioned you" in that account's inbox and makes them a | |
| 523 | participant of the thread, provided they can read the repository and | |
| 524 | have not muted it; watchers are not told about a mention addressed to | |
| 525 | someone else. | |
| 521 | 526 | You are never mailed about your own actions, and only verified primary |
| 522 | 527 | addresses receive anything. Delivery retries on relay failure. |
| 523 | 528 | |
e2e/mentions_test.go added +92
| @@ -0,0 +1,92 @@ | ||
| 1 | package e2e | |
| 2 | ||
| 3 | import ( | |
| 4 | "strings" | |
| 5 | "testing" | |
| 6 | ) | |
| 7 | ||
| 8 | // An @mention files an inbox row for the mentioned account and makes them | |
| 9 | // a participant of the thread; watchers are not told about the mention, | |
| 10 | // mute holds, and someone who cannot read the repository is not reached | |
| 11 | // (#202). | |
| 12 | func TestMentionsNotify(t *testing.T) { | |
| 13 | inst := startInstance(t) | |
| 14 | keys := map[string]string{} | |
| 15 | for _, u := range []string{"alice", "bob", "carol", "eve"} { | |
| 16 | keys[u] = inst.newKey(t, u) | |
| 17 | inst.admin(t, "admin", "user", "create", u, "--key", keys[u]+".pub") | |
| 18 | } | |
| 19 | if _, errOut, code := inst.ssh(t, keys["alice"], "", "repo", "create", "alice/app"); code != 0 { | |
| 20 | t.Fatalf("repo create: %s", errOut) | |
| 21 | } | |
| 22 | if _, _, code := inst.ssh(t, keys["carol"], "", "repo", "watch", "alice/app"); code != 0 { | |
| 23 | t.Fatal("watch failed") | |
| 24 | } | |
| 25 | summaries := func(key string) []string { | |
| 26 | var out []string | |
| 27 | for _, n := range notices(t, inst, key) { | |
| 28 | out = append(out, n.Summary) | |
| 29 | } | |
| 30 | return out | |
| 31 | } | |
| 32 | has := func(list []string, want string) bool { | |
| 33 | for _, s := range list { | |
| 34 | if strings.Contains(s, want) { | |
| 35 | return true | |
| 36 | } | |
| 37 | } | |
| 38 | return false | |
| 39 | } | |
| 40 | ||
| 41 | // A mention in an issue body, with prose punctuation after the name. | |
| 42 | if _, errOut, code := inst.ssh(t, keys["alice"], "", "issue", "create", "alice/app", | |
| 43 | "--title", "'leak'", "--body", "'@bob, can you look? cc @nobody and @alice.'"); code != 0 { | |
| 44 | t.Fatalf("issue create: %s", errOut) | |
| 45 | } | |
| 46 | if got := summaries(keys["bob"]); !has(got, "mentioned you in #1") { | |
| 47 | t.Fatalf("bob not told about the mention: %v", got) | |
| 48 | } | |
| 49 | if got := summaries(keys["carol"]); has(got, "mentioned") || !has(got, "opened issue #1") { | |
| 50 | t.Fatalf("watcher inbox: %v", got) | |
| 51 | } | |
| 52 | if got := summaries(keys["alice"]); has(got, "mentioned") { | |
| 53 | t.Fatalf("self-mention filed: %v", got) | |
| 54 | } | |
| 55 | ||
| 56 | // bob is now a participant: a later comment reaches him. | |
| 57 | if _, errOut, code := inst.ssh(t, keys["alice"], "", "issue", "comment", "alice/app", "1", "--message", "'more'"); code != 0 { | |
| 58 | t.Fatalf("comment: %s", errOut) | |
| 59 | } | |
| 60 | if got := summaries(keys["bob"]); !has(got, "commented on #1") { | |
| 61 | t.Fatalf("mentioned account not a participant: %v", got) | |
| 62 | } | |
| 63 | ||
| 64 | // Muted, a mention does not get through. | |
| 65 | if _, _, code := inst.ssh(t, keys["bob"], "", "repo", "mute", "alice/app"); code != 0 { | |
| 66 | t.Fatal("mute failed") | |
| 67 | } | |
| 68 | if _, errOut, code := inst.ssh(t, keys["alice"], "", "issue", "comment", "alice/app", "1", "--message", "'@bob again'"); code != 0 { | |
| 69 | t.Fatalf("comment: %s", errOut) | |
| 70 | } | |
| 71 | if got := summaries(keys["bob"]); has(got, "mentioned you in #1") && len(got) > 2 { | |
| 72 | t.Fatalf("mention reached a muted account: %v", got) | |
| 73 | } | |
| 74 | ||
| 75 | // A private repository: a mention of someone who cannot read it is | |
| 76 | // dropped, one of someone who can is filed. Merge request bodies too. | |
| 77 | for _, args := range [][]string{ | |
| 78 | {"repo", "create", "alice/secret", "--private"}, | |
| 79 | {"repo", "access", "grant", "alice/secret", "carol", "read"}, | |
| 80 | {"issue", "create", "alice/secret", "--title", "'hush'", "--body", "'@eve @carol'"}, | |
| 81 | } { | |
| 82 | if _, errOut, code := inst.ssh(t, keys["alice"], "", args...); code != 0 { | |
| 83 | t.Fatalf("%v: %s", args, errOut) | |
| 84 | } | |
| 85 | } | |
| 86 | if got := summaries(keys["eve"]); len(got) != 0 { | |
| 87 | t.Fatalf("outsider reached through a private repository: %v", got) | |
| 88 | } | |
| 89 | if got := summaries(keys["carol"]); !has(got, "mentioned you in #1") { | |
| 90 | t.Fatalf("reader of a private repository not told: %v", got) | |
| 91 | } | |
| 92 | } | |
internal/autolink/autolink.go +16
| @@ -84,6 +84,22 @@ type span struct { | ||
| 84 | 84 | |
| 85 | 85 | // rewriteText returns replacement nodes for a text node, or nil when no |
| 86 | 86 | // reference resolved. |
| 87 | // Mentions returns the distinct @names in text, in order of appearance, | |
| 88 | // as written. A name may carry trailing punctuation the writer meant as | |
| 89 | // prose ("@alice."); the caller resolves and, failing that, trims ._- | |
| 90 | // the way Rewrite does. | |
| 91 | func Mentions(text string) []string { | |
| 92 | seen := map[string]bool{} | |
| 93 | var out []string | |
| 94 | for _, m := range mentionPat.FindAllStringSubmatch(text, -1) { | |
| 95 | if who := m[2]; !seen[who] { | |
| 96 | seen[who] = true | |
| 97 | out = append(out, who) | |
| 98 | } | |
| 99 | } | |
| 100 | return out | |
| 101 | } | |
| 102 | ||
| 87 | 103 | func rewriteText(text, owner, name string, r Resolver) []*html.Node { |
| 88 | 104 | var spans []span |
| 89 | 105 | |
internal/autolink/autolink_test.go +13
| @@ -100,3 +100,16 @@ func TestRewriteEscaping(t *testing.T) { | ||
| 100 | 100 | t.Fatalf("ref not linked:\n%s", got) |
| 101 | 101 | } |
| 102 | 102 | } |
| 103 | ||
| 104 | func TestMentions(t *testing.T) { | |
| 105 | got := Mentions("cc @alice and (@bob) — @alice again; mail@example.org is not one, @carol.") | |
| 106 | want := []string{"alice", "bob", "carol."} | |
| 107 | if len(got) != len(want) { | |
| 108 | t.Fatalf("Mentions = %v, want %v", got, want) | |
| 109 | } | |
| 110 | for i := range want { | |
| 111 | if got[i] != want[i] { | |
| 112 | t.Fatalf("Mentions = %v, want %v", got, want) | |
| 113 | } | |
| 114 | } | |
| 115 | } | |
internal/control/diffcomment.go +1
| @@ -113,6 +113,7 @@ func runDiffComment(c *Ctx, args []string) int { | ||
| 113 | 113 | action: fmt.Sprintf("commented on %s:%d in !%d", path, line, mr.Number), |
| 114 | 114 | excerpt: body, path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)}) |
| 115 | 115 | } |
| 116 | notifyMentions(c, repo, mrThread, mr.ID, mr.Number, mr.Title, body) | |
| 116 | 117 | } |
| 117 | 118 | return c.emit(map[string]any{"id": id, "thread": firstNonZero(replyTo, id), "pending": pending}, func(w io.Writer) { |
| 118 | 119 | what := "thread %d opened on %s:%d in %s!%d\n" |
internal/control/issue.go +3
| @@ -155,6 +155,9 @@ func runIssueCreate(c *Ctx, args []string) int { | ||
| 155 | 155 | action: fmt.Sprintf("opened issue #%d", n), |
| 156 | 156 | excerpt: b, path: fmt.Sprintf("%s/issues/%d", repo.Path(), n)}) |
| 157 | 157 | } |
| 158 | if issue, err := c.Store.IssueByNumber(repo.ID, n); err == nil { | |
| 159 | notifyMentions(c, repo, issueThread, issue.ID, n, title, b) | |
| 160 | } | |
| 158 | 161 | return c.emit(Created{Number: n}, func(w io.Writer) { |
| 159 | 162 | fmt.Fprintf(w, "created %s#%d\n", repo.Path(), n) |
| 160 | 163 | }) |
internal/control/mr.go +3
| @@ -344,6 +344,9 @@ func runMRCreate(c *Ctx, args []string) int { | ||
| 344 | 344 | action: fmt.Sprintf("opened merge request !%d (%s -> %s)", n, source, target), |
| 345 | 345 | excerpt: b, path: fmt.Sprintf("%s/mrs/%d", repo.Path(), n)}) |
| 346 | 346 | } |
| 347 | if created, err := c.Store.MRByNumber(repo.ID, n); err == nil { | |
| 348 | notifyMentions(c, repo, mrThread, created.ID, n, title, b) | |
| 349 | } | |
| 347 | 350 | out := MRCreated{Number: n, HeadSHA: headSHA} |
| 348 | 351 | if p, ok, err := c.Store.OpenMRBySource(repo.ID, target); err == nil && ok { |
| 349 | 352 | out.StackedOn = &stackRef{p.Number, p.Title} |
internal/control/notifications.go +44 −4
| @@ -6,6 +6,7 @@ import ( | ||
| 6 | 6 | "strconv" |
| 7 | 7 | "strings" |
| 8 | 8 | |
| 9 | "gitbay.org/gitbay/internal/autolink" | |
| 9 | 10 | "gitbay.org/gitbay/internal/policy" |
| 10 | 11 | "gitbay.org/gitbay/internal/protocol" |
| 11 | 12 | "gitbay.org/gitbay/internal/store" |
| @@ -44,14 +45,17 @@ type notice struct { | ||
| 44 | 45 | // mail is not prose — a failed build's log tail is not an excerpt of |
| 45 | 46 | // something someone wrote, and is not cut to an excerpt's length. |
| 46 | 47 | body string |
| 48 | // direct keeps the notice to the given accounts: watchers of the | |
| 49 | // repository are not added. A mention is addressed to someone. | |
| 50 | direct bool | |
| 47 | 51 | } |
| 48 | 52 | |
| 49 | 53 | // notify delivers a notice to the given user ids widened by the |
| 50 | // repository's watchers, minus anyone who muted it and minus the acting | |
| 51 | // user. A best-effort side channel: failures are ignored, the action | |
| 52 | // itself already succeeded. | |
| 54 | // repository's watchers (unless direct), minus anyone who muted it and | |
| 55 | // minus the acting user. A best-effort side channel: failures are | |
| 56 | // ignored, the action itself already succeeded. | |
| 53 | 57 | func notify(c *Ctx, userIDs []int64, n notice) { |
| 54 | recipients, err := c.Store.NotifyRecipients(n.repo.ID, c.User.ID, userIDs) | |
| 58 | recipients, err := c.Store.NotifyRecipients(n.repo.ID, c.User.ID, userIDs, !n.direct) | |
| 55 | 59 | if err != nil { |
| 56 | 60 | return |
| 57 | 61 | } |
| @@ -70,6 +74,42 @@ func notify(c *Ctx, userIDs []int64, n notice) { | ||
| 70 | 74 | } |
| 71 | 75 | } |
| 72 | 76 | |
| 77 | // notifyMentions files an inbox row for every account text mentions by | |
| 78 | // @name that can read the repository, and records them as participants | |
| 79 | // of the thread so they hear what follows (#202). Mute is honoured by | |
| 80 | // notify; the actor mentioning themselves is dropped there too. | |
| 81 | func notifyMentions(c *Ctx, repo store.Repo, t thread, itemID, number int64, title, text string) { | |
| 82 | var ids []int64 | |
| 83 | for _, name := range autolink.Mentions(text) { | |
| 84 | u, err := c.Store.UserByUsername(name) | |
| 85 | if err != nil { | |
| 86 | trimmed := strings.TrimRight(name, "._-") | |
| 87 | if trimmed == "" || trimmed == name { | |
| 88 | continue | |
| 89 | } | |
| 90 | if u, err = c.Store.UserByUsername(trimmed); err != nil { | |
| 91 | continue | |
| 92 | } | |
| 93 | } | |
| 94 | if u.ID == c.User.ID { | |
| 95 | continue | |
| 96 | } | |
| 97 | grant, err := c.Store.AccessRole(repo.ID, u.ID) | |
| 98 | if err != nil || !policy.CanRead(u, repo, grant) { | |
| 99 | continue | |
| 100 | } | |
| 101 | ids = append(ids, u.ID) | |
| 102 | } | |
| 103 | if len(ids) == 0 { | |
| 104 | return | |
| 105 | } | |
| 106 | c.Store.AddMentions(repo.ID, t.kind, itemID, ids) | |
| 107 | notify(c, ids, notice{repo: repo, kind: t.kind, direct: true, | |
| 108 | subject: fmt.Sprintf("[%s] %s%d: %s", repo.Path(), t.symbol, number, title), | |
| 109 | action: fmt.Sprintf("mentioned you in %s%d", t.symbol, number), | |
| 110 | excerpt: text, path: fmt.Sprintf("%s/%s/%d", repo.Path(), t.segment, number)}) | |
| 111 | } | |
| 112 | ||
| 73 | 113 | // noticeBody builds the standard mail body: who did what, an excerpt, and |
| 74 | 114 | // the web link. |
| 75 | 115 | func noticeBody(c *Ctx, n notice) string { |
internal/control/thread.go +1
| @@ -112,6 +112,7 @@ func runComment(c *Ctx, args []string, t thread, noun string, | ||
| 112 | 112 | action: fmt.Sprintf("commented on %s%d", t.symbol, number), |
| 113 | 113 | excerpt: body, path: fmt.Sprintf("%s/%s/%d", repo.Path(), t.segment, number)}) |
| 114 | 114 | } |
| 115 | notifyMentions(c, repo, t, id, number, title, body) | |
| 115 | 116 | return c.emit(Created{Number: number}, func(w io.Writer) { |
| 116 | 117 | fmt.Fprintf(w, "commented on %s%s%d\n", repo.Path(), t.symbol, number) |
| 117 | 118 | }) |
internal/deps/worker.go +1 −1
| @@ -238,7 +238,7 @@ func (w *Worker) notify(repo store.Repo, number int64, action, body string) { | ||
| 238 | 238 | if err != nil { |
| 239 | 239 | return |
| 240 | 240 | } |
| 241 | recipients, err := w.St.NotifyRecipients(repo.ID, author.ID, targets) | |
| 241 | recipients, err := w.St.NotifyRecipients(repo.ID, author.ID, targets, true) | |
| 242 | 242 | if err != nil { |
| 243 | 243 | return |
| 244 | 244 | } |
internal/store/inbox.go +4 −1
| @@ -131,7 +131,7 @@ func (s *Store) RepoWatchState(repoID, userID int64) string { | ||
| 131 | 131 | // the repository's watchers, minus the actor and minus anyone who muted |
| 132 | 132 | // it. Muting wins over every other reason to be told, including owning |
| 133 | 133 | // the repository or having written the thread. |
| 134 | func (s *Store) NotifyRecipients(repoID, actorID int64, targets []int64) ([]int64, error) { | |
| 134 | func (s *Store) NotifyRecipients(repoID, actorID int64, targets []int64, widen bool) ([]int64, error) { | |
| 135 | 135 | rows, err := s.DB.Query("SELECT user_id, state FROM repo_watchers WHERE repo_id = ?", repoID) |
| 136 | 136 | if err != nil { |
| 137 | 137 | return nil, err |
| @@ -156,6 +156,9 @@ func (s *Store) NotifyRecipients(repoID, actorID int64, targets []int64) ([]int6 | ||
| 156 | 156 | } |
| 157 | 157 | var out []int64 |
| 158 | 158 | seen := map[int64]bool{} |
| 159 | if !widen { | |
| 160 | watching = nil | |
| 161 | } | |
| 159 | 162 | for _, id := range append(append([]int64{}, targets...), watching...) { |
| 160 | 163 | if skip[id] || seen[id] { |
| 161 | 164 | continue |
internal/store/inbox_test.go +9 −5
| @@ -115,7 +115,7 @@ func TestNotifyRecipients(t *testing.T) { | ||
| 115 | 115 | t.Fatal(err) |
| 116 | 116 | } |
| 117 | 117 | |
| 118 | got, err := s.NotifyRecipients(repoID, other, []int64{owner, other}) | |
| 118 | got, err := s.NotifyRecipients(repoID, other, []int64{owner, other}, true) | |
| 119 | 119 | if err != nil { |
| 120 | 120 | t.Fatal(err) |
| 121 | 121 | } |
| @@ -126,20 +126,24 @@ func TestNotifyRecipients(t *testing.T) { | ||
| 126 | 126 | if err := s.SetRepoWatch(repoID, third, "watching"); err != nil { |
| 127 | 127 | t.Fatal(err) |
| 128 | 128 | } |
| 129 | if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}); len(got) != 2 { | |
| 129 | if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}, true); len(got) != 2 { | |
| 130 | 130 | t.Fatalf("watcher not added: %v", got) |
| 131 | 131 | } |
| 132 | 132 | |
| 133 | 133 | // A watcher who is also a target is listed once. |
| 134 | if got, _ := s.NotifyRecipients(repoID, other, []int64{owner, third}); len(got) != 2 { | |
| 134 | if got, _ := s.NotifyRecipients(repoID, other, []int64{owner, third}, true); len(got) != 2 { | |
| 135 | 135 | t.Fatalf("watcher duplicated: %v", got) |
| 136 | 136 | } |
| 137 | // A direct notice stays with its targets; watchers are not added. | |
| 138 | if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}, false); len(got) != 1 || got[0] != owner { | |
| 139 | t.Fatalf("direct notice widened: %v", got) | |
| 140 | } | |
| 137 | 141 | |
| 138 | 142 | // Muting beats owning the repository. |
| 139 | 143 | if err := s.SetRepoWatch(repoID, owner, "muted"); err != nil { |
| 140 | 144 | t.Fatal(err) |
| 141 | 145 | } |
| 142 | got, _ = s.NotifyRecipients(repoID, other, []int64{owner}) | |
| 146 | got, _ = s.NotifyRecipients(repoID, other, []int64{owner}, true) | |
| 143 | 147 | if len(got) != 1 || got[0] != third { |
| 144 | 148 | t.Fatalf("muted owner still notified: %v", got) |
| 145 | 149 | } |
| @@ -153,7 +157,7 @@ func TestNotifyRecipients(t *testing.T) { | ||
| 153 | 157 | if s.RepoWatchState(repoID, owner) != "" { |
| 154 | 158 | t.Fatal("clear left a state") |
| 155 | 159 | } |
| 156 | if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}); len(got) != 2 { | |
| 160 | if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}, true); len(got) != 2 { | |
| 157 | 161 | t.Fatalf("cleared owner not notified: %v", got) |
| 158 | 162 | } |
| 159 | 163 | if err := s.SetRepoWatch(repoID, owner, "muted"); err != nil { |
internal/store/migrations/0047_mentions.down.sql added +1
| @@ -0,0 +1 @@ | ||
| 1 | DROP TABLE mentions; | |
internal/store/migrations/0047_mentions.up.sql added +11
| @@ -0,0 +1,11 @@ | ||
| 1 | -- Who an issue or merge request body, or a comment on one, mentioned by | |
| 2 | -- @name. A mentioned account is a participant of the thread from then | |
| 3 | -- on, the way an author or commenter is (#202). repo_id carries the | |
| 4 | -- cascade, since item_id is polymorphic. | |
| 5 | CREATE TABLE mentions ( | |
| 6 | repo_id INTEGER NOT NULL REFERENCES repos(id) ON DELETE CASCADE, | |
| 7 | kind TEXT NOT NULL CHECK (kind IN ('issue', 'mr')), | |
| 8 | item_id INTEGER NOT NULL, | |
| 9 | user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, | |
| 10 | PRIMARY KEY (kind, item_id, user_id) | |
| 11 | ); | |
internal/store/notify.go +19 −4
| @@ -58,21 +58,36 @@ func (s *Store) MarkMailFailed(id int64, errMsg string, nextAt *time.Time) error | ||
| 58 | 58 | } |
| 59 | 59 | |
| 60 | 60 | // IssueParticipants returns distinct user ids involved in an issue: the |
| 61 | // author and every commenter. | |
| 61 | // author, every commenter, and everyone mentioned. | |
| 62 | 62 | func (s *Store) IssueParticipants(issueID int64) ([]int64, error) { |
| 63 | 63 | return s.idQuery(` |
| 64 | 64 | SELECT author_id FROM issues WHERE id = ? |
| 65 | UNION SELECT author_id FROM issue_comments WHERE issue_id = ?`, issueID, issueID) | |
| 65 | UNION SELECT author_id FROM issue_comments WHERE issue_id = ? | |
| 66 | UNION SELECT user_id FROM mentions WHERE kind = 'issue' AND item_id = ?`, issueID, issueID, issueID) | |
| 66 | 67 | } |
| 67 | 68 | |
| 68 | 69 | // MRParticipants returns distinct user ids involved in an MR: author, |
| 69 | // commenters, reviewers. | |
| 70 | // commenters, reviewers, and everyone mentioned. | |
| 70 | 71 | func (s *Store) MRParticipants(mrID int64) ([]int64, error) { |
| 71 | 72 | return s.idQuery(` |
| 72 | 73 | SELECT author_id FROM merge_requests WHERE id = ? |
| 73 | 74 | UNION SELECT author_id FROM mr_comments WHERE mr_id = ? |
| 74 | 75 | UNION SELECT reviewer_id FROM mr_reviews WHERE mr_id = ? |
| 75 | UNION SELECT author_id FROM mr_diff_comments WHERE mr_id = ?`, mrID, mrID, mrID, mrID) | |
| 76 | UNION SELECT author_id FROM mr_diff_comments WHERE mr_id = ? | |
| 77 | UNION SELECT user_id FROM mentions WHERE kind = 'mr' AND item_id = ?`, mrID, mrID, mrID, mrID, mrID) | |
| 78 | } | |
| 79 | ||
| 80 | // AddMentions records accounts mentioned in an issue or merge request | |
| 81 | // (kind "issue" or "mr"); a repeat mention is not an error. | |
| 82 | func (s *Store) AddMentions(repoID int64, kind string, itemID int64, userIDs []int64) error { | |
| 83 | for _, id := range userIDs { | |
| 84 | if _, err := s.DB.Exec( | |
| 85 | "INSERT INTO mentions (repo_id, kind, item_id, user_id) VALUES (?, ?, ?, ?) ON CONFLICT DO NOTHING", | |
| 86 | repoID, kind, itemID, id); err != nil { | |
| 87 | return err | |
| 88 | } | |
| 89 | } | |
| 90 | return nil | |
| 76 | 91 | } |
| 77 | 92 | |
| 78 | 93 | // RepoNotifyTargets returns who should hear about new activity on a repo: |