krz/gitbay

store, control, web, wiki: notifications settings mail on|off !345

merged merged by cmc on 2026-09-08 02:35 UTC · krz/gitbay:mail-preference into main

13 files changed, +250 −76

Layout: unified · split

.gitbay/wiki/Parity.org +1
@@ -277,6 +277,7 @@ client has no use for one (krz/gitbay#57).
277| email list, remove, primary | yes | yes | no | 277| email list, remove, primary | yes | yes | no |
278| dashboard aggregate | yes | yes | yes | 278| dashboard aggregate | yes | yes | yes |
279| notification inbox | yes | yes | yes | 279| notification inbox | yes | yes | yes |
280| activity mail on, off | yes | yes | no |
280| API token mint | yes | no | no | 281| API token mint | yes | no | no |
281| account export bundle | yes | yes | n/a | 282| account export bundle | yes | yes | n/a |
282| profile set | yes | yes | yes | 283| profile set | yes | yes | yes |
.gitbay/wiki/Users.org +6 −4
@@ -515,10 +515,12 @@ handled.
515 515
516* Notifications 516* Notifications
517 517
518When the instance has SMTP configured, activity mails you: someone 518When the instance has SMTP configured, activity mails you as well as
519opens an issue or MR on your repository, comments where you are a 519filing the inbox row: someone opens an issue or MR on your repository,
520participant (author, commenter, reviewer, or mentioned), reviews, 520comments where you are a participant (author, commenter, reviewer, or
521closes, or merges. Writing =@name= in an issue, merge request or 521mentioned), reviews, closes, or merges. =notifications settings mail
522off= keeps the inbox and stops that mail (login links are not activity
523and still arrive); the account page has the same switch. Writing =@name= in an issue, merge request or
522comment files "mentioned you" in that account's inbox and makes them a 524comment files "mentioned you" in that account's inbox and makes them a
523participant of the thread, provided they can read the repository and 525participant of the thread, provided they can read the repository and
524have not muted it; watchers are not told about a mention addressed to 526have not muted it; watchers are not told about a mention addressed to
cmd/gitbay/main.go +4
@@ -66,6 +66,10 @@ func newRoot() *cobra.Command {
66 passOpts{server: []string{"notifications", "list"}}), 66 passOpts{server: []string{"notifications", "list"}}),
67 pass("read", "mark notifications read: <id>... | --all", 67 pass("read", "mark notifications read: <id>... | --all",
68 passOpts{server: []string{"notifications", "read"}}), 68 passOpts{server: []string{"notifications", "read"}}),
69 group("settings", "notification preferences",
70 pass("show", "your notification preferences", passOpts{server: []string{"notifications", "settings", "show"}}),
71 pass("mail", "activity by mail as well as the inbox: on|off", passOpts{server: []string{"notifications", "settings", "mail"}}),
72 ),
69 ), 73 ),
70 group("wiki", "a repository's wiki pages", 74 group("wiki", "a repository's wiki pages",
71 pass("list", "list pages: [<owner/name>]", passOpts{server: []string{"wiki", "list"}, needsRepo: true}), 75 pass("list", "list pages: [<owner/name>]", passOpts{server: []string{"wiki", "list"}, needsRepo: true}),
e2e/mailpref_test.go added +69
@@ -0,0 +1,69 @@
1package e2e
2
3import (
4 "fmt"
5 "net/url"
6 "strings"
7 "testing"
8 "time"
9)
10
11// notifications settings mail off keeps the inbox and stops activity
12// mail; on brings it back. The account page carries the same switch
13// (#194).
14func TestMailPreference(t *testing.T) {
15 smtp := startFakeSMTP(t)
16 inst := startInstanceWith(t, fmt.Sprintf(
17 "[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n[web]\nmode = \"accounts\"\n", smtp.addr))
18 aliceKey := inst.newKey(t, "alice")
19 bobKey := inst.newKey(t, "bob")
20 inst.admin(t, "admin", "user", "create", "alice",
21 "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
22 inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub")
23 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 {
24 t.Fatalf("repo create: %s", errOut)
25 }
26
27 out, _, code := inst.ssh(t, aliceKey, "", "notifications", "settings", "show", "--json")
28 if code != 0 || !strings.Contains(out, `"mail":true`) {
29 t.Fatalf("settings show: %d %s", code, out)
30 }
31 if _, _, code := inst.ssh(t, aliceKey, "", "notifications", "settings", "mail", "sometimes"); code != 2 {
32 t.Fatalf("bad value accepted: %d", code)
33 }
34 if out, _, code := inst.ssh(t, aliceKey, "", "notifications", "settings", "mail", "off", "--json"); code != 0 || !strings.Contains(out, `"mail":false`) {
35 t.Fatalf("mail off: %d %s", code, out)
36 }
37
38 // Mail off: the inbox row is filed, no mail goes out.
39 if _, errOut, code := inst.ssh(t, bobKey, "", "issue", "create", "alice/app", "--title", "'leak'"); code != 0 {
40 t.Fatalf("issue create: %s", errOut)
41 }
42 rows := notices(t, inst, aliceKey)
43 if len(rows) != 1 || !strings.Contains(rows[0].Summary, "opened issue #1") {
44 t.Fatalf("inbox with mail off: %+v", rows)
45 }
46 time.Sleep(5 * time.Second) // the mailer ticks every two seconds
47 if got := smtp.mailTo("alice@example.test"); len(got) != 0 {
48 t.Fatalf("mail sent with the preference off:\n%s", got[0])
49 }
50
51 // The account page shows it off and turns it back on.
52 alice := inst.login(t, aliceKey)
53 set := inst.base() + "/settings"
54 if _, body := browserGet(t, alice, set); !strings.Contains(body, `name="mail" value="on">`) || strings.Contains(body, `name="mail" value="on" checked`) {
55 t.Fatalf("account page does not show mail off:\n%s", body)
56 }
57 if status, _ := browserPost(t, alice, set, url.Values{"field": {"notify-mail"}, "mail": {"on"}}); status != 200 {
58 t.Fatalf("settings post: %d", status)
59 }
60 if out, _, _ := inst.ssh(t, aliceKey, "", "notifications", "settings", "show", "--json"); !strings.Contains(out, `"mail":true`) {
61 t.Fatalf("web toggle did not turn mail on: %s", out)
62 }
63 if _, errOut, code := inst.ssh(t, bobKey, "", "issue", "create", "alice/app", "--title", "'still leaking'"); code != 0 {
64 t.Fatalf("second issue: %s", errOut)
65 }
66 if m := smtp.waitFor(t, "alice@example.test", "still leaking"); !strings.Contains(m, "bob opened issue #2") {
67 t.Fatalf("mail after turning it on:\n%s", m)
68 }
69}
e2e/readonly_test.go +70 −69
@@ -83,75 +83,76 @@ func TestReadOnlyCommandsWriteNothing(t *testing.T) {
83 must("", "webhook", "add", "alice/app", "http://127.0.0.1:1/hook", "--events", "release.created") 83 must("", "webhook", "add", "alice/app", "http://127.0.0.1:1/hook", "--events", "release.created")
84 84
85 readArgs := map[string][]string{ 85 readArgs := map[string][]string{
86 "help": {}, 86 "help": {},
87 "whoami": {}, 87 "whoami": {},
88 "dashboard": {}, 88 "dashboard": {},
89 "feed": {}, 89 "feed": {},
90 "explore": {}, 90 "explore": {},
91 "audit": {}, 91 "audit": {},
92 "keys list": {}, 92 "keys list": {},
93 "email list": {}, 93 "email list": {},
94 "pgp list": {}, 94 "pgp list": {},
95 "token list": {}, 95 "token list": {},
96 "web sessions list": {}, 96 "web sessions list": {},
97 "account export": {}, 97 "account export": {},
98 "org list": {}, 98 "org list": {},
99 "repo list": {}, 99 "repo list": {},
100 "admin user list": {}, 100 "admin user list": {},
101 "admin runners": {}, 101 "admin runners": {},
102 "admin repo list": {}, 102 "admin repo list": {},
103 "admin stats": {}, 103 "admin stats": {},
104 "admin user show": {"alice"}, 104 "admin user show": {"alice"},
105 "profile show": {"alice"}, 105 "profile show": {"alice"},
106 "org show": {"theorg"}, 106 "org show": {"theorg"},
107 "org members list": {"theorg"}, 107 "org members list": {"theorg"},
108 "org team list": {"theorg"}, 108 "org team list": {"theorg"},
109 "org team show": {"theorg", "core"}, 109 "org team show": {"theorg", "core"},
110 "repo search": {"app"}, 110 "repo search": {"app"},
111 "repo show": {"alice/app"}, 111 "repo show": {"alice/app"},
112 "repo access list": {"alice/app"}, 112 "repo access list": {"alice/app"},
113 "repo settings show": {"alice/app"}, 113 "repo settings show": {"alice/app"},
114 "repo topics": {"alice/app"}, 114 "repo topics": {"alice/app"},
115 "repo refs": {"alice/app"}, 115 "repo refs": {"alice/app"},
116 "repo log": {"alice/app"}, 116 "repo log": {"alice/app"},
117 "repo tree": {"alice/app"}, 117 "repo tree": {"alice/app"},
118 "repo cat": {"alice/app", "f.go"}, 118 "repo cat": {"alice/app", "f.go"},
119 "repo blame": {"alice/app", "f.go"}, 119 "repo blame": {"alice/app", "f.go"},
120 "repo grep": {"alice/app", "hello"}, 120 "repo grep": {"alice/app", "hello"},
121 "repo diff": {"alice/app", "main", "feat"}, 121 "repo diff": {"alice/app", "main", "feat"},
122 "repo commit": {"alice/app", sha}, 122 "repo commit": {"alice/app", sha},
123 "repo download": {"alice/app"}, 123 "repo download": {"alice/app"},
124 "repo deploy-key list": {"alice/app"}, 124 "repo deploy-key list": {"alice/app"},
125 "repo secret list": {"alice/app"}, 125 "repo secret list": {"alice/app"},
126 "repo mirror list": {"alice/app"}, 126 "repo mirror list": {"alice/app"},
127 "repo domain list": {"alice/app"}, 127 "repo domain list": {"alice/app"},
128 "repo deps status": {"alice/app"}, 128 "repo deps status": {"alice/app"},
129 "status list": {"alice/app", sha}, 129 "status list": {"alice/app", sha},
130 "issue list": {"alice/app"}, 130 "issue list": {"alice/app"},
131 "issue show": {"alice/app", "1"}, 131 "issue show": {"alice/app", "1"},
132 "issue templates": {"alice/app"}, 132 "issue templates": {"alice/app"},
133 "label list": {"alice/app"}, 133 "label list": {"alice/app"},
134 "milestone list": {"alice/app"}, 134 "milestone list": {"alice/app"},
135 "mr list": {"alice/app"}, 135 "mr list": {"alice/app"},
136 "mr show": {"alice/app", "1"}, 136 "mr show": {"alice/app", "1"},
137 "mr diff": {"alice/app", "1"}, 137 "mr diff": {"alice/app", "1"},
138 "mr threads": {"alice/app", "1"}, 138 "mr threads": {"alice/app", "1"},
139 "build list": {"alice/app"}, 139 "build list": {"alice/app"},
140 "build jobs": {"alice/app"}, 140 "build jobs": {"alice/app"},
141 "build show": {"alice/app", "1"}, 141 "build show": {"alice/app", "1"},
142 "build log": {"alice/app", "1"}, 142 "build log": {"alice/app", "1"},
143 "release list": {"alice/app"}, 143 "release list": {"alice/app"},
144 "release show": {"alice/app", "v1"}, 144 "release show": {"alice/app", "v1"},
145 "release asset get": {"alice/app", "v1", "a.txt"}, 145 "release asset get": {"alice/app", "v1", "a.txt"},
146 "notifications list": nil, 146 "notifications list": nil,
147 "repo bookmarks": nil, 147 "notifications settings show": nil,
148 "search": {"app"}, 148 "repo bookmarks": nil,
149 "mr revisions": {"alice/app", "1"}, 149 "search": {"app"},
150 "mr range-diff": {"alice/app", "1"}, 150 "mr revisions": {"alice/app", "1"},
151 "webhook list": {"alice/app"}, 151 "mr range-diff": {"alice/app", "1"},
152 "webhook deliveries": {"alice/app"}, 152 "webhook list": {"alice/app"},
153 "wiki list": {"alice/app"}, 153 "webhook deliveries": {"alice/app"},
154 "wiki show": {"alice/app"}, 154 "wiki list": {"alice/app"},
155 "wiki show": {"alice/app"},
155 } 156 }
156 // Reads whose subject legitimately does not exist in this fixture. 157 // Reads whose subject legitimately does not exist in this fixture.
157 notFoundOK := map[string]bool{"wiki show": true, "repo deps status": true} 158 notFoundOK := map[string]bool{"wiki show": true, "repo deps status": true}
internal/control/notifications.go +41 −1
@@ -20,6 +20,13 @@ func init() {
20 register(Command{Path: []string{"notifications", "read"}, 20 register(Command{Path: []string{"notifications", "read"},
21 Summary: "mark notifications read", 21 Summary: "mark notifications read",
22 Usage: "notifications read <id>... | --all", Run: runNotificationsRead}) 22 Usage: "notifications read <id>... | --all", Run: runNotificationsRead})
23 register(Command{Path: []string{"notifications", "settings", "show"},
24 Summary: "your notification preferences",
25 Usage: "notifications settings show",
26 ReadOnly: true, Run: runNotificationsSettingsShow})
27 register(Command{Path: []string{"notifications", "settings", "mail"},
28 Summary: "activity by mail as well as the inbox (login links are unaffected)",
29 Usage: "notifications settings mail on|off", Run: runNotificationsSettingsMail})
23 register(Command{Path: []string{"repo", "watch"}, 30 register(Command{Path: []string{"repo", "watch"},
24 Summary: "hear about all activity on a repository", 31 Summary: "hear about all activity on a repository",
25 Usage: "repo watch <owner/name>", Run: runRepoWatch}) 32 Usage: "repo watch <owner/name>", Run: runRepoWatch})
@@ -66,7 +73,7 @@ func notify(c *Ctx, userIDs []int64, n notice) {
66 if !sendMail { 73 if !sendMail {
67 continue 74 continue
68 } 75 }
69 email, err := c.Store.PrimaryVerifiedEmail(id) 76 email, err := c.Store.ActivityMailAddress(id)
70 if err != nil || email == "" { 77 if err != nil || email == "" {
71 continue 78 continue
72 } 79 }
@@ -136,6 +143,39 @@ func mrSubject(repo store.Repo, number int64, title string) string {
136 return fmt.Sprintf("[%s] !%d: %s", repo.Path(), number, title) 143 return fmt.Sprintf("[%s] !%d: %s", repo.Path(), number, title)
137} 144}
138 145
146func emitNotificationSettings(c *Ctx) int {
147 on, err := c.Store.MailEnabled(c.User.ID)
148 if err != nil {
149 return c.fail(protocol.ExitFailure, "%v", err)
150 }
151 return c.emit(map[string]bool{"mail": on}, func(w io.Writer) {
152 state := "off"
153 if on {
154 state = "on"
155 }
156 fmt.Fprintf(w, "mail: %s\n", state)
157 })
158}
159
160func runNotificationsSettingsShow(c *Ctx, args []string) int {
161 if len(args) != 0 {
162 return c.fail(protocol.ExitUsage, "usage: notifications settings show")
163 }
164 return emitNotificationSettings(c)
165}
166
167// runNotificationsSettingsMail is the "inbox but no mail" switch: the
168// inbox is filed either way, the mail half consults it (#194).
169func runNotificationsSettingsMail(c *Ctx, args []string) int {
170 if len(args) != 1 || (args[0] != "on" && args[0] != "off") {
171 return c.fail(protocol.ExitUsage, "usage: notifications settings mail on|off")
172 }
173 if err := c.Store.SetMailEnabled(c.User.ID, args[0] == "on"); err != nil {
174 return c.fail(protocol.ExitFailure, "%v", err)
175 }
176 return emitNotificationSettings(c)
177}
178
139// noticesDefaultLimit caps a bare list; pagination reaches further back. 179// noticesDefaultLimit caps a bare list; pagination reaches further back.
140const noticesDefaultLimit = 50 180const noticesDefaultLimit = 50
141 181
internal/deps/worker.go +1 −1
@@ -251,7 +251,7 @@ func (w *Worker) notify(repo store.Repo, number int64, action, body string) {
251 if w.Cfg.Mail.SMTPHost == "" { 251 if w.Cfg.Mail.SMTPHost == "" {
252 continue 252 continue
253 } 253 }
254 email, err := w.St.PrimaryVerifiedEmail(id) 254 email, err := w.St.ActivityMailAddress(id)
255 if err != nil || email == "" { 255 if err != nil || email == "" {
256 continue 256 continue
257 } 257 }
internal/httpd/account.go +13 −1
@@ -52,6 +52,7 @@ func (s *Server) accountForm(w http.ResponseWriter, r *http.Request, u store.Use
52 52
53 var profile control.ProfileOut 53 var profile control.ProfileOut
54 s.runControlInto(u, []string{"profile", "show"}, &profile) 54 s.runControlInto(u, []string{"profile", "show"}, &profile)
55 mailOn, _ := s.st.MailEnabled(u.ID)
55 56
56 s.render(w, "account.html", struct { 57 s.render(w, "account.html", struct {
57 basePage 58 basePage
@@ -64,8 +65,9 @@ func (s *Server) accountForm(w http.ResponseWriter, r *http.Request, u store.Use
64 Host string 65 Host string
65 Notice string 66 Notice string
66 Message string 67 Message string
68 MailOn bool
67 }{s.baseFor(u), "account", keys, pgp, emails, profile, profileLinksText(profile.Links), s.cfg.SiteHost(), 69 }{s.baseFor(u), "account", keys, pgp, emails, profile, profileLinksText(profile.Links), s.cfg.SiteHost(),
68 s.takeFlash(w, r), r.URL.Query().Get("m")}) 70 s.takeFlash(w, r), r.URL.Query().Get("m"), mailOn})
69} 71}
70 72
71// accountExport hands the browser the same bundle `account export` 73// accountExport hands the browser the same bundle `account export`
@@ -191,6 +193,16 @@ func (s *Server) accountSubmit(w http.ResponseWriter, r *http.Request, u store.U
191 return 193 return
192 } 194 }
193 back("", "primary address changed") 195 back("", "primary address changed")
196 case "notify-mail":
197 state := "off"
198 if r.FormValue("mail") == "on" {
199 state = "on"
200 }
201 if _, msg, ok := s.runControl(u, []string{"notifications", "settings", "mail", state}); !ok {
202 back(msg, "")
203 return
204 }
205 back("", "notification preferences saved")
194 case "profile": 206 case "profile":
195 format := r.FormValue("format") 207 format := r.FormValue("format")
196 if format != "org" { 208 if format != "org" {
internal/store/migrations/0048_notify_mail.down.sql added +1
@@ -0,0 +1 @@
1ALTER TABLE users DROP COLUMN notify_mail;
internal/store/migrations/0048_notify_mail.up.sql added +4
@@ -0,0 +1,4 @@
1-- Whether activity notifications reach the account by mail as well as
2-- the inbox. Off leaves inbox rows untouched and skips the mail half
3-- only; login links and verification mail are not activity (#194).
4ALTER TABLE users ADD COLUMN notify_mail INTEGER NOT NULL DEFAULT 1;
internal/store/mrs.go +11
@@ -494,6 +494,17 @@ func (s *Store) PrimaryVerifiedEmail(userID int64) (string, error) {
494 return addr, err 494 return addr, err
495} 495}
496 496
497// ActivityMailAddress returns where activity mail for an account goes:
498// its verified primary address, or "" when there is none or the account
499// turned activity mail off (#194).
500func (s *Store) ActivityMailAddress(userID int64) (string, error) {
501 on, err := s.MailEnabled(userID)
502 if err != nil || !on {
503 return "", err
504 }
505 return s.PrimaryVerifiedEmail(userID)
506}
507
497// PreferredVerifiedEmail returns the primary address if it is verified, 508// PreferredVerifiedEmail returns the primary address if it is verified,
498// otherwise the account's other verified address that sorts first by 509// otherwise the account's other verified address that sorts first by
499// address; "" if none is verified. Unlike PrimaryVerifiedEmail, a verified 510// address; "" if none is verified. Unlike PrimaryVerifiedEmail, a verified
internal/store/users.go +20
@@ -195,6 +195,26 @@ func (s *Store) SetUserDisabled(userID int64, disabled bool) error {
195 return err 195 return err
196} 196}
197 197
198// MailEnabled reports whether activity notifications reach the account
199// by mail as well as the inbox.
200func (s *Store) MailEnabled(userID int64) (bool, error) {
201 var on int
202 err := s.DB.QueryRow("SELECT notify_mail FROM users WHERE id = ?", userID).Scan(&on)
203 if errors.Is(err, sql.ErrNoRows) {
204 return false, ErrNotFound
205 }
206 return on != 0, err
207}
208
209func (s *Store) SetMailEnabled(userID int64, on bool) error {
210 v := 0
211 if on {
212 v = 1
213 }
214 _, err := s.DB.Exec("UPDATE users SET notify_mail = ? WHERE id = ?", v, userID)
215 return err
216}
217
198func (s *Store) UserByID(id int64) (User, error) { 218func (s *Store) UserByID(id int64) (User, error) {
199 var u User 219 var u User
200 var admin, pending, disabled int 220 var admin, pending, disabled int
internal/web/templates/account.html +9
@@ -98,6 +98,15 @@ account, and where notifications go.</p>
98 </form> 98 </form>
99</details> 99</details>
100 100
101<h2>Notifications</h2>
102<form method="post" action="/settings" class="setform">
103 <input type="hidden" name="field" value="notify-mail">
104 <label for="notify-mail">Activity by mail</label>
105 <input type="checkbox" id="notify-mail" name="mail" value="on"{{if .MailOn}} checked{{end}}>
106 <button type="submit">Save</button>
107</form>
108<p class="meta">The inbox is filed either way; this is the mail half. Login links are not activity and still arrive.</p>
109
101<h2>Export</h2> 110<h2>Export</h2>
102<p class="meta">Your profile, repositories, issues and merge requests as one 111<p class="meta">Your profile, repositories, issues and merge requests as one
103JSON bundle, the same one <code>gitbay account export</code> writes. Keys are 112JSON bundle, the same one <code>gitbay account export</code> writes. Keys are