diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 7a54390f076..0c2c3bfc45b 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -477,7 +477,7 @@ func applySubscribedCondition(sess db.Session, subscriberID int64) { Select("repo_id"). From("watch"). Where(builder.And(builder.Eq{"user_id": subscriberID}, - builder.In("mode", repo_model.WatchModeNormal, repo_model.WatchModeAuto))), + builder.In("mode", repo_model.WatchModeNormal, repo_model.WatchModeAuto, repo_model.WatchModeCustom))), ), ), ) diff --git a/models/repo/watch.go b/models/repo/watch.go index f8c413b14b2..ab8e9129dfa 100644 --- a/models/repo/watch.go +++ b/models/repo/watch.go @@ -20,9 +20,10 @@ type WatchMode int8 const ( WatchModeNone WatchMode = iota // 0 watch nothing unless mentioned - WatchModeNormal // 1 proactively watching (all or custom) + WatchModeNormal // 1 proactively watching (all, or custom before WatchModeCustom) WatchModeDont // 2 ignore the repo WatchModeAuto // 3 automatically watching (from AutoWatchOnChanges) + WatchModeCustom // 4 watching the events picked in the custom watch options ) // WatchType is the `watch` column gating one kind of notification @@ -79,7 +80,7 @@ func (w *Watch) IsWatching() bool { // IsWatchingAll reports whether every event is enabled, which is the "all activity" mode func (w *Watch) IsWatchingAll() bool { - return w.IncludePullRequests && w.IncludeIssues && w.IncludeReleases + return w.Mode != WatchModeCustom && w.IncludePullRequests && w.IncludeIssues && w.IncludeReleases } func (w *Watch) IsWatchingAny() bool { @@ -166,6 +167,9 @@ func WatchRepoAuto(ctx context.Context, doer *user_model.User, repo *Repository, if user_model.IsUserBlockedBy(ctx, doer, repo.OwnerID) { return user_model.ErrBlockedUser } + if watch.Mode == WatchModeCustom { // keep the events the user picked + return nil + } return watchRepoByMode(ctx, watch, WatchModeNormal) } @@ -189,7 +193,7 @@ func WatchRepoWithOptions(ctx context.Context, doer *user_model.User, repo *Repo if err != nil { return err } - if opts.Mode == WatchModeNormal { + if opts.Mode == WatchModeNormal || opts.Mode == WatchModeCustom { _, err = db.GetEngine(ctx).Where("user_id=? AND repo_id=?", doer.ID, repo.ID). Cols("include_pull_requests", "include_issues", "include_releases"). Update(&Watch{IncludePullRequests: opts.WatchPullRequests, IncludeIssues: opts.WatchIssues, IncludeReleases: opts.WatchReleases}) @@ -247,6 +251,16 @@ func GetRepoIgnorersIDs(ctx context.Context, repoID int64) ([]int64, error) { // but avoids joining with `user` for performance reasons // User permissions must be verified elsewhere if required func GetRepoWatchersIDs(ctx context.Context, repoID int64, watchType WatchType) ([]int64, error) { + return getRepoWatchersIDs(ctx, repoID, watchType, false) +} + +// GetRepoCustomWatchersIDs is GetRepoWatchersIDs limited to custom watches, which unlike +// watching all activity are never set up automatically (e.g. by AutoWatchNewRepos) +func GetRepoCustomWatchersIDs(ctx context.Context, repoID int64, watchType WatchType) ([]int64, error) { + return getRepoWatchersIDs(ctx, repoID, watchType, true) +} + +func getRepoWatchersIDs(ctx context.Context, repoID int64, watchType WatchType, customOnly bool) ([]int64, error) { ids := make([]int64, 0, 64) var watchColName string switch watchType { @@ -259,12 +273,19 @@ func GetRepoWatchersIDs(ctx context.Context, repoID int64, watchType WatchType) default: panic("invalid WatchType") } - return ids, db.GetEngine(ctx).Table("watch"). + sess := db.GetEngine(ctx).Table("watch"). Where("watch.repo_id=?", repoID). And("watch.mode<>?", WatchModeDont). - And(builder.Eq{watchColName: true}). - Select("user_id"). - Find(&ids) + And(builder.Eq{watchColName: true}) + if customOnly { // matches SelectedMode() == "custom" + sess = sess.And(builder.Or( + builder.Eq{"watch.mode": WatchModeCustom}, + builder.Eq{"watch.include_pull_requests": false}, + builder.Eq{"watch.include_issues": false}, + builder.Eq{"watch.include_releases": false}, + )) + } + return ids, sess.Select("user_id").Find(&ids) } // GetRepoWatchers returns range of users watching given repository. diff --git a/models/repo/watch_test.go b/models/repo/watch_test.go index 104154102e7..b21f3b7ec3b 100644 --- a/models/repo/watch_test.go +++ b/models/repo/watch_test.go @@ -145,7 +145,7 @@ func TestWatchOptions(t *testing.T) { // repo 1 is watched by users 1, 4, 9 and 11, all with every event enabled repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - assert.NoError(t, repo_model.WatchRepoWithOptions(t.Context(), user, repo, repo_model.WatchOptions{Mode: repo_model.WatchModeNormal, WatchPullRequests: true})) + assert.NoError(t, repo_model.WatchRepoWithOptions(t.Context(), user, repo, repo_model.WatchOptions{Mode: repo_model.WatchModeNormal, WatchPullRequests: true})) // custom watches were stored like this before WatchModeCustom for watchType, expected := range map[repo_model.WatchType][]int64{ repo_model.WatchPullRequests: {1, 4, 9, 11}, @@ -157,6 +157,20 @@ func TestWatchOptions(t *testing.T) { assert.ElementsMatch(t, expected, ids, watchType) } + // a custom watch picking every event differs from all activity, which can be set up automatically + customUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + assert.NoError(t, repo_model.WatchRepoWithOptions(t.Context(), customUser, repo, repo_model.WatchOptions{Mode: repo_model.WatchModeCustom, WatchPullRequests: true, WatchIssues: true, WatchReleases: true})) + assert.NoError(t, repo_model.WatchRepoAuto(t.Context(), customUser, repo, true)) // must keep the custom watch + for watchType, expected := range map[repo_model.WatchType][]int64{ + repo_model.WatchPullRequests: {1, 4}, + repo_model.WatchIssues: {4}, + repo_model.WatchReleases: {4}, + } { + ids, err := repo_model.GetRepoCustomWatchersIDs(t.Context(), repo.ID, watchType) + assert.NoError(t, err) + assert.ElementsMatch(t, expected, ids, watchType) + } + // the options of one user must not show up for another watches, err := repo_model.GetUserWatches(t.Context(), 4, []int64{repo.ID}) assert.NoError(t, err) @@ -176,5 +190,6 @@ func TestWatchSelectedMode(t *testing.T) { assert.Equal(t, "participate", (&repo_model.Watch{Mode: repo_model.WatchModeNormal}).SelectedMode()) assert.Equal(t, "ignore", (&repo_model.Watch{Mode: repo_model.WatchModeDont}).SelectedMode()) assert.Equal(t, "custom", (&repo_model.Watch{Mode: repo_model.WatchModeNormal, IncludeIssues: true}).SelectedMode()) + assert.Equal(t, "custom", (&repo_model.Watch{Mode: repo_model.WatchModeCustom, IncludePullRequests: true, IncludeIssues: true, IncludeReleases: true}).SelectedMode()) assert.Equal(t, "all", (&repo_model.Watch{Mode: repo_model.WatchModeAuto, IncludePullRequests: true, IncludeIssues: true, IncludeReleases: true}).SelectedMode()) } diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index b66f4b3dc07..b0b5868a228 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -930,7 +930,7 @@ "settings.delete_account_title": "Delete User Account", "settings.delete_account_desc": "Are you sure you want to permanently delete this user account?", "settings.email_notifications.enable": "Enable Email Notifications", - "settings.email_notifications.onmention": "Only Email on Mention", + "settings.email_notifications.onmention_and_custom": "Only Email on Mention and Custom Watches", "settings.email_notifications.disable": "Disable Email Notifications", "settings.email_notifications.submit": "Set Email Preference", "settings.email_notifications.andyourown": "And Your Own Notifications", diff --git a/routers/web/repo/watch.go b/routers/web/repo/watch.go index adf2d38b7a8..92a9fe9ed3b 100644 --- a/routers/web/repo/watch.go +++ b/routers/web/repo/watch.go @@ -49,7 +49,7 @@ func ActionWatch(ctx *context.Context) { // ActionWatchOptions watches the repository with a custom selection of events func ActionWatchOptions(ctx *context.Context) { opts := repo_model.WatchOptions{ // clearing every event is allowed, it leaves the participating state - Mode: repo_model.WatchModeNormal, + Mode: repo_model.WatchModeCustom, WatchPullRequests: ctx.FormBool("pull_requests"), WatchIssues: ctx.FormBool("issues"), WatchReleases: ctx.FormBool("releases"), diff --git a/services/mailer/mail_comment.go b/services/mailer/mail_comment.go index 2e564fcb8f1..209597af587 100644 --- a/services/mailer/mail_comment.go +++ b/services/mailer/mail_comment.go @@ -54,7 +54,7 @@ func MailMentionsComment(ctx context.Context, pr *issues_model.PullRequest, c *i ActionType: activities_model.ActionCommentPull, Content: c.Content, Comment: c, - }, mentions, visited, true); err != nil { + }, mentions, visited, true, nil); err != nil { log.Error("mailIssueCommentBatch: %v", err) } return nil diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index 85eabb4a37b..e29d76ad9b4 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -66,6 +66,7 @@ func mailIssueCommentToParticipants(ctx context.Context, comment *mailComment, m // =========== Repo watchers =========== // Make repo watchers last, since it's likely the list with the most users + var customWatchers container.Set[int64] if !(comment.Issue.IsPull && comment.Issue.PullRequest.IsWorkInProgress(ctx) && comment.ActionType != activities_model.ActionCreatePullRequest) { watchType := util.Iif(comment.Issue.IsPull, repo_model.WatchPullRequests, repo_model.WatchIssues) ids, err = repo_model.GetRepoWatchersIDs(ctx, comment.Issue.RepoID, watchType) @@ -73,6 +74,12 @@ func mailIssueCommentToParticipants(ctx context.Context, comment *mailComment, m return fmt.Errorf("GetRepoWatchersIDs(%d): %w", comment.Issue.RepoID, err) } unfiltered = append(ids, unfiltered...) + + ids, err = repo_model.GetRepoCustomWatchersIDs(ctx, comment.Issue.RepoID, watchType) + if err != nil { + return fmt.Errorf("GetRepoCustomWatchersIDs(%d): %w", comment.Issue.RepoID, err) + } + customWatchers = container.SetOf(ids...) } visited := make(container.Set[int64], len(unfiltered)+len(mentions)+1) @@ -90,7 +97,7 @@ func mailIssueCommentToParticipants(ctx context.Context, comment *mailComment, m } // =========== Mentions =========== - if err = mailIssueCommentBatch(ctx, comment, mentions, visited, true); err != nil { + if err = mailIssueCommentBatch(ctx, comment, mentions, visited, true, nil); err != nil { return fmt.Errorf("mailIssueCommentBatch() mentions: %w", err) } @@ -101,18 +108,18 @@ func mailIssueCommentToParticipants(ctx context.Context, comment *mailComment, m } visited.AddMultiple(ids...) - unfilteredUsers, err := user_model.GetMailableUsersByIDs(ctx, unfiltered, false) + unfilteredUsers, err := user_model.GetMailableUsersByIDs(ctx, unfiltered, len(customWatchers) > 0) if err != nil { return err } - if err = mailIssueCommentBatch(ctx, comment, unfilteredUsers, visited, false); err != nil { + if err = mailIssueCommentBatch(ctx, comment, unfilteredUsers, visited, false, customWatchers); err != nil { return fmt.Errorf("mailIssueCommentBatch(): %w", err) } return nil } -func mailIssueCommentBatch(ctx context.Context, comment *mailComment, users []*user_model.User, visited container.Set[int64], fromMention bool) error { +func mailIssueCommentBatch(ctx context.Context, comment *mailComment, users []*user_model.User, visited container.Set[int64], fromMention bool, customWatchers container.Set[int64]) error { checkUnit := unit.TypeIssues if comment.Issue.IsPull { checkUnit = unit.TypePullRequests @@ -124,10 +131,10 @@ func mailIssueCommentBatch(ctx context.Context, comment *mailComment, users []*u continue } // At this point we exclude: - // user that don't have all mails enabled or users only get mail on mention and this is one ... + // user that don't have all mails enabled or users only get mail on mention or custom watch and this is neither ... if !(user.EmailNotificationsPreference == user_model.EmailNotificationsEnabled || user.EmailNotificationsPreference == user_model.EmailNotificationsAndYourOwn || - fromMention && user.EmailNotificationsPreference == user_model.EmailNotificationsOnMention) { + user.EmailNotificationsPreference == user_model.EmailNotificationsOnMention && (fromMention || customWatchers.Contains(user.ID))) { continue } diff --git a/services/mailer/mail_release.go b/services/mailer/mail_release.go index ced05c29159..85dd9b046d8 100644 --- a/services/mailer/mail_release.go +++ b/services/mailer/mail_release.go @@ -14,6 +14,7 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" user_model "gitea.dev/models/user" + "gitea.dev/modules/container" "gitea.dev/modules/log" "gitea.dev/modules/markup/markdown" "gitea.dev/modules/setting" @@ -41,7 +42,14 @@ func MailNewRelease(ctx context.Context, rel *repo_model.Release) { return } - recipients, err := user_model.GetMailableUsersByIDs(ctx, watcherIDList, false) + customWatcherIDs, err := repo_model.GetRepoCustomWatchersIDs(ctx, rel.RepoID, repo_model.WatchReleases) + if err != nil { + log.Error("GetRepoCustomWatchersIDs(%d): %v", rel.RepoID, err) + return + } + customWatchers := container.SetOf(customWatcherIDs...) + + recipients, err := user_model.GetMailableUsersByIDs(ctx, watcherIDList, len(customWatchers) > 0) if err != nil { log.Error("user_model.GetMailableUsersByIDs: %v", err) return @@ -52,9 +60,11 @@ func MailNewRelease(ctx context.Context, rel *repo_model.Release) { return } - // delete publisher or any users with no permission + // delete publisher, on-mention users without a custom watch or any users with no permission recipients = slices.DeleteFunc(recipients, func(u *user_model.User) bool { - return u.ID == rel.PublisherID || !access_model.CheckRepoUnitUser(ctx, rel.Repo, u, unit.TypeReleases) + return u.ID == rel.PublisherID || + u.EmailNotificationsPreference == user_model.EmailNotificationsOnMention && !customWatchers.Contains(u.ID) || + !access_model.CheckRepoUnitUser(ctx, rel.Repo, u, unit.TypeReleases) }) langMap := make(map[string][]*user_model.User) diff --git a/services/mailer/mail_release_test.go b/services/mailer/mail_release_test.go index a2ed0072c1a..5e38e588862 100644 --- a/services/mailer/mail_release_test.go +++ b/services/mailer/mail_release_test.go @@ -61,3 +61,29 @@ func TestMailNewReleaseFiltersUnauthorizedWatchers(t *testing.T) { assert.Equal(t, admin.EmailTo(), sent[0].To) assert.NotEqual(t, unauthorized.EmailTo(), sent[0].To) } + +func TestMailNewReleaseOnMentionCustomWatch(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&setting.MailService, &setting.Mailer{FromEmail: "noreply@example.com"})() + defer mockMailTemplates(string(tplNewReleaseMail), "{{.Subject}}", "

{{.Release.TagName}}

")() + + // users 4 and 9 only get mail on mention and watch repo 1 with all activity + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + customWatcher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + allWatcher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 9}) + require.NoError(t, repo_model.WatchRepoWithOptions(t.Context(), customWatcher, repo, repo_model.WatchOptions{Mode: repo_model.WatchModeCustom, WatchReleases: true})) + + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: 1, RepoID: repo.ID}) + rel.Publisher = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: rel.PublisherID}) + + var recipients []string + defer test.MockVariableValue(&SendAsync, func(msgs ...*sender_service.Message) { + for _, msg := range msgs { + recipients = append(recipients, msg.To) + } + })() + + MailNewRelease(t.Context(), rel) + assert.Contains(t, recipients, customWatcher.EmailTo()) + assert.NotContains(t, recipients, allWatcher.EmailTo()) +} diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index 03c7f5f8258..eab9a97c306 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -167,6 +167,28 @@ func TestMailMentionsComment(t *testing.T) { assert.Equal(t, 3, mails) } +func TestMailParticipantsOnMentionCustomWatch(t *testing.T) { + doer, repo, issue, comment := prepareMailerTest(t) + comment.Poster = doer + defer mockMailTemplates("mail/repo/issue/comment", subjectTpl, bodyTpl)() + + // users 4 and 9 only get mail on mention and watch repo 1 with all activity + customWatcher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + allWatcher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 9}) + require.NoError(t, repo_model.WatchRepoWithOptions(t.Context(), customWatcher, repo, repo_model.WatchOptions{Mode: repo_model.WatchModeCustom, WatchPullRequests: true, WatchIssues: true, WatchReleases: true})) + + var recipients []string + defer test.MockVariableValue(&SendAsync, func(msgs ...*sender_service.Message) { + for _, msg := range msgs { + recipients = append(recipients, msg.To) + } + })() + + require.NoError(t, MailParticipantsComment(t.Context(), comment, activities_model.ActionCommentIssue, issue, nil)) + assert.Contains(t, recipients, customWatcher.Email) + assert.NotContains(t, recipients, allWatcher.Email) +} + func TestMailsSkipBots(t *testing.T) { doer, repo, issue, comment := prepareMailerTest(t) comment.Poster = doer diff --git a/templates/user/settings/notifications.tmpl b/templates/user/settings/notifications.tmpl index e205b1af650..bcdf8de0b88 100644 --- a/templates/user/settings/notifications.tmpl +++ b/templates/user/settings/notifications.tmpl @@ -16,7 +16,7 @@