From f1c414882cbbdb22c6bcc6315c03a1d3c8454164 Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Thu, 29 Aug 2019 14:05:42 +0000 Subject: [PATCH] Add Ability for User to Customize Email Notification Frequency (#7813) * Add Backend Logic for Toggling Email Notification This commit adds the backend logic for allowing users to enable or disable email notifications. The implementation ensures that only issue notification emails get disabled and important emails are still sent regardless of the setting. The UI to toggle this setting has not yet been implemented. * Add UI and complete user email notification enable This commit completes the functionality to allow users to disable their own email notifications. Signed-off-by: Gary Kim * Add Third Option for Only Email on Mention Signed-off-by: Gary Kim * Readd NOT NULL to new preference string Signed-off-by: Gary Kim * Add Tests and Rewrite Comment Signed-off-by: Gary Kim * Allow admin to set default email frequency Signed-off-by: Gary Kim * Add new config option to docs Signed-off-by: Gary Kim * Fix a few mistakes Signed-off-by: Gary Kim * Only update required columns Signed-off-by: Gary Kim * Simplify an error check Signed-off-by: Gary Kim * Make email_notification_preference column in DB be VARCHAR(20) Signed-off-by: Gary Kim * Handle errors Signed-off-by: Gary Kim * Update models/migrations/v93.go Co-Authored-By: Lauris BH --- custom/conf/app.ini.sample | 2 + .../doc/advanced/config-cheat-sheet.en-us.md | 3 ++ models/fixtures/user.yml | 9 +++++ models/issue_mail.go | 8 ++-- models/migrations/migrations.go | 2 + models/migrations/v93.go | 16 ++++++++ models/user.go | 37 ++++++++++++++++--- models/user_test.go | 33 +++++++++++++++++ modules/setting/setting.go | 4 ++ options/locale/locale_en-US.ini | 9 +++++ public/css/index.css | 2 +- public/less/_repository.less | 2 +- routers/user/setting/account.go | 22 +++++++++++ templates/user/settings/account.tmpl | 27 +++++++++++++- 14 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 models/migrations/v93.go diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index 82ffd2d78..a2ac7248e 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -306,6 +306,8 @@ MAX_FILE_SIZE = 1048576 [admin] ; Disallow regular (non-admin) users from creating organizations. DISABLE_REGULAR_ORG_CREATION = false +; Default configuration for email notifications for users (user configurable). Options: enabled, onmention, disabled +DEFAULT_EMAIL_NOTIFICATIONS = enabled [security] ; Whether the installer is disabled diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 252ceee73..10c6110f3 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -184,6 +184,9 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `UPDATE_BUFFER_LEN`: **20**: Buffer length of index request. - `MAX_FILE_SIZE`: **1048576**: Maximum size in bytes of files to be indexed. +## Admin (`admin`) +- `DEFAULT_EMAIL_NOTIFICATIONS`: **enabled**: Default configuration for email notifications for users (user configurable). Options: enabled, onmention, disabled + ## Security (`security`) - `INSTALL_LOCK`: **false**: Disallow access to the install page. diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index d89dc3c12..37225f144 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -6,6 +6,7 @@ name: user1 full_name: User One email: user1@example.com + email_notifications_preference: enabled passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual salt: ZogKvWdyEx @@ -22,6 +23,7 @@ full_name: " < Ur Tw >< " email: user2@example.com keep_email_private: true + email_notifications_preference: enabled passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual salt: ZogKvWdyEx @@ -40,6 +42,7 @@ name: user3 full_name: " <<<< >> >> > >> > >>> >> " email: user3@example.com + email_notifications_preference: onmention passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 1 # organization salt: ZogKvWdyEx @@ -56,6 +59,7 @@ name: user4 full_name: " " email: user4@example.com + email_notifications_preference: onmention passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual salt: ZogKvWdyEx @@ -72,6 +76,7 @@ name: user5 full_name: User Five email: user5@example.com + email_notifications_preference: enabled passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual salt: ZogKvWdyEx @@ -89,6 +94,7 @@ name: user6 full_name: User Six email: user6@example.com + email_notifications_preference: enabled passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 1 # organization salt: ZogKvWdyEx @@ -105,6 +111,7 @@ name: user7 full_name: User Seven email: user7@example.com + email_notifications_preference: disabled passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 1 # organization salt: ZogKvWdyEx @@ -121,6 +128,7 @@ name: user8 full_name: User Eight email: user8@example.com + email_notifications_preference: enabled passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual salt: ZogKvWdyEx @@ -138,6 +146,7 @@ name: user9 full_name: User Nine email: user9@example.com + email_notifications_preference: onmention passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual salt: ZogKvWdyEx diff --git a/models/issue_mail.go b/models/issue_mail.go index 829c4cea7..87d991e50 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -70,7 +70,7 @@ func mailIssueCommentToParticipants(e Engine, issue *Issue, doer *User, content if err != nil { return fmt.Errorf("GetUserByID [%d]: %v", watchers[i].UserID, err) } - if to.IsOrganization() { + if to.IsOrganization() || to.EmailNotifications() != EmailNotificationsEnabled { continue } @@ -78,9 +78,9 @@ func mailIssueCommentToParticipants(e Engine, issue *Issue, doer *User, content names = append(names, to.Name) } for i := range participants { - if participants[i].ID == doer.ID { - continue - } else if com.IsSliceContainsStr(names, participants[i].Name) { + if participants[i].ID == doer.ID || + com.IsSliceContainsStr(names, participants[i].Name) || + participants[i].EmailNotifications() != EmailNotificationsEnabled { continue } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 7d97741a2..15e021c05 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -240,6 +240,8 @@ var migrations = []Migration{ NewMigration("add index on owner_id of repository and type, review_id of comment", addIndexOnRepositoryAndComment), // v92 -> v93 NewMigration("remove orphaned repository index statuses", removeLingeringIndexStatus), + // v93 -> v94 + NewMigration("add email notification enabled preference to user", addEmailNotificationEnabledToUser), } // Migrate database to current version diff --git a/models/migrations/v93.go b/models/migrations/v93.go new file mode 100644 index 000000000..0b0441cd5 --- /dev/null +++ b/models/migrations/v93.go @@ -0,0 +1,16 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import "github.com/go-xorm/xorm" + +func addEmailNotificationEnabledToUser(x *xorm.Engine) error { + // User see models/user.go + type User struct { + EmailNotificationsPreference string `xorm:"VARCHAR(20) NOT NULL DEFAULT 'enabled'"` + } + + return x.Sync2(new(User)) +} diff --git a/models/user.go b/models/user.go index ed0fe524e..af4ccacf6 100644 --- a/models/user.go +++ b/models/user.go @@ -58,6 +58,13 @@ const ( algoScrypt = "scrypt" algoArgon2 = "argon2" algoPbkdf2 = "pbkdf2" + + // EmailNotificationsEnabled indicates that the user would like to receive all email notifications + EmailNotificationsEnabled = "enabled" + // EmailNotificationsOnMention indicates that the user would like to be notified via email when mentioned. + EmailNotificationsOnMention = "onmention" + // EmailNotificationsDisabled indicates that the user would not like to be notified via email. + EmailNotificationsDisabled = "disabled" ) var ( @@ -87,10 +94,11 @@ type User struct { Name string `xorm:"UNIQUE NOT NULL"` FullName string // Email is the primary email address (to be used for communication) - Email string `xorm:"NOT NULL"` - KeepEmailPrivate bool - Passwd string `xorm:"NOT NULL"` - PasswdHashAlgo string `xorm:"NOT NULL DEFAULT 'pbkdf2'"` + Email string `xorm:"NOT NULL"` + KeepEmailPrivate bool + EmailNotificationsPreference string `xorm:"VARCHAR(20) NOT NULL DEFAULT 'enabled'"` + Passwd string `xorm:"NOT NULL"` + PasswdHashAlgo string `xorm:"NOT NULL DEFAULT 'pbkdf2'"` // MustChangePassword is an attribute that determines if a user // is to change his/her password after registration. @@ -719,6 +727,21 @@ func (u *User) IsMailable() bool { return u.IsActive } +// EmailNotifications returns the User's email notification preference +func (u *User) EmailNotifications() string { + return u.EmailNotificationsPreference +} + +// SetEmailNotifications sets the user's email notification preference +func (u *User) SetEmailNotifications(set string) error { + u.EmailNotificationsPreference = set + if err := UpdateUserCols(u, "email_notifications_preference"); err != nil { + log.Error("SetEmailNotifications: %v", err) + return err + } + return nil +} + func isUserExist(e Engine, uid int64, name string) (bool, error) { if len(name) == 0 { return false, nil @@ -868,6 +891,7 @@ func CreateUser(u *User) (err error) { } u.HashPassword(u.Passwd) u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization && !setting.Admin.DisableRegularOrgCreation + u.EmailNotificationsPreference = setting.Admin.DefaultEmailNotification u.MaxRepoCreation = -1 u.Theme = setting.UI.DefaultTheme @@ -1253,7 +1277,8 @@ func getUserByName(e Engine, name string) (*User, error) { return u, nil } -// GetUserEmailsByNames returns a list of e-mails corresponds to names. +// GetUserEmailsByNames returns a list of e-mails corresponds to names of users +// that have their email notifications set to enabled or onmention. func GetUserEmailsByNames(names []string) []string { return getUserEmailsByNames(x, names) } @@ -1265,7 +1290,7 @@ func getUserEmailsByNames(e Engine, names []string) []string { if err != nil { continue } - if u.IsMailable() { + if u.IsMailable() && u.EmailNotifications() != EmailNotificationsDisabled { mails = append(mails, u.Email) } } diff --git a/models/user_test.go b/models/user_test.go index 290253c4b..d01b482ae 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -74,6 +74,8 @@ func TestGetUserEmailsByNames(t *testing.T) { // ignore none active user email assert.Equal(t, []string{"user8@example.com"}, GetUserEmailsByNames([]string{"user8", "user9"})) assert.Equal(t, []string{"user8@example.com", "user5@example.com"}, GetUserEmailsByNames([]string{"user8", "user5"})) + + assert.Equal(t, []string{"user8@example.com"}, GetUserEmailsByNames([]string{"user8", "user7"})) } func TestUser_APIFormat(t *testing.T) { @@ -196,6 +198,37 @@ func TestDeleteUser(t *testing.T) { test(11) } +func TestEmailNotificationPreferences(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + for _, test := range []struct { + expected string + userID int64 + }{ + {EmailNotificationsEnabled, 1}, + {EmailNotificationsEnabled, 2}, + {EmailNotificationsOnMention, 3}, + {EmailNotificationsOnMention, 4}, + {EmailNotificationsEnabled, 5}, + {EmailNotificationsEnabled, 6}, + {EmailNotificationsDisabled, 7}, + {EmailNotificationsEnabled, 8}, + {EmailNotificationsOnMention, 9}, + } { + user := AssertExistsAndLoadBean(t, &User{ID: test.userID}).(*User) + assert.Equal(t, test.expected, user.EmailNotifications()) + + // Try all possible settings + assert.NoError(t, user.SetEmailNotifications(EmailNotificationsEnabled)) + assert.Equal(t, EmailNotificationsEnabled, user.EmailNotifications()) + + assert.NoError(t, user.SetEmailNotifications(EmailNotificationsOnMention)) + assert.Equal(t, EmailNotificationsOnMention, user.EmailNotifications()) + + assert.NoError(t, user.SetEmailNotifications(EmailNotificationsDisabled)) + assert.Equal(t, EmailNotificationsDisabled, user.EmailNotifications()) + } +} + func TestHashPasswordDeterministic(t *testing.T) { b := make([]byte, 16) rand.Read(b) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 3ff9f89ad..5e476854b 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -231,6 +231,7 @@ var ( // Admin settings Admin struct { DisableRegularOrgCreation bool + DefaultEmailNotification string } // Picture settings @@ -754,6 +755,9 @@ func NewContext() { } } + sec = Cfg.Section("admin") + Admin.DefaultEmailNotification = sec.Key("DEFAULT_EMAIL_NOTIFICATIONS").MustString("enabled") + sec = Cfg.Section("security") InstallLock = sec.Key("INSTALL_LOCK").MustBool(false) SecretKey = sec.Key("SECRET_KEY").MustString("!#@FDEWREWR&*(") diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index d6d54a224..90ee32a25 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -557,6 +557,11 @@ confirm_delete_account = Confirm Deletion delete_account_title = Delete User Account delete_account_desc = Are you sure you want to permanently delete this user account? +email_notifications.enable = Enable Email Notifications +email_notifications.onmention = Only Email on Mention +email_notifications.disable = Disable Email Notifications +email_notifications.submit = Set Email Preference + [repo] owner = Owner repo_name = Repository Name @@ -1126,6 +1131,10 @@ settings.basic_settings = Basic Settings settings.mirror_settings = Mirror Settings settings.sync_mirror = Synchronize Now settings.mirror_sync_in_progress = Mirror synchronization is in progress. Check back in a minute. +settings.email_notifications.enable = Enable Email Notifications +settings.email_notifications.onmention = Only Email on Mention +settings.email_notifications.disable = Disable Email Notifications +settings.email_notifications.submit = Set Email Preference settings.site = Website settings.update_settings = Update Settings settings.advanced_settings = Advanced Settings diff --git a/public/css/index.css b/public/css/index.css index ad1d0313f..a60963758 100644 --- a/public/css/index.css +++ b/public/css/index.css @@ -786,7 +786,7 @@ footer .ui.left,footer .ui.right{line-height:40px} .ui.form .dropzone .dz-error-message{top:140px} .settings .content{margin-top:2px} .settings .content .segment,.settings .content>.header{box-shadow:0 1px 2px 0 rgba(34,36,38,.15)} -.settings .list>.item .green{color:#21ba45} +.settings .list>.item .green:not(.ui.button){color:#21ba45} .settings .list>.item:not(:first-child){border-top:1px solid #eaeaea;padding:1rem;margin:15px -1rem -1rem -1rem} .settings .list>.item>.mega-octicon{display:table-cell} .settings .list>.item>.mega-octicon+.content{display:table-cell;padding:0 0 0 .5em;vertical-align:top} diff --git a/public/less/_repository.less b/public/less/_repository.less index ef05beb6f..a66d13891 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -2013,7 +2013,7 @@ .list { > .item { - .green { + .green:not(.ui.button) { color: #21ba45; } diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go index c6b5c0c2d..674606cce 100644 --- a/routers/user/setting/account.go +++ b/routers/user/setting/account.go @@ -6,6 +6,8 @@ package setting import ( + "errors" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/base" @@ -24,6 +26,7 @@ func Account(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true ctx.Data["Email"] = ctx.User.Email + ctx.Data["EmailNotificationsPreference"] = ctx.User.EmailNotifications() loadAccountData(ctx) @@ -82,6 +85,25 @@ func EmailPost(ctx *context.Context, form auth.AddEmailForm) { ctx.Redirect(setting.AppSubURL + "/user/settings/account") return } + // Set Email Notification Preference + if ctx.Query("_method") == "NOTIFICATION" { + preference := ctx.Query("preference") + if !(preference == models.EmailNotificationsEnabled || + preference == models.EmailNotificationsOnMention || + preference == models.EmailNotificationsDisabled) { + log.Error("Email notifications preference change returned unrecognized option %s: %s", preference, ctx.User.Name) + ctx.ServerError("SetEmailPreference", errors.New("option unrecognized")) + return + } + if err := ctx.User.SetEmailNotifications(preference); err != nil { + log.Error("Set Email Notifications failed: %v", err) + ctx.ServerError("SetEmailNotifications", err) + return + } + log.Trace("Email notifications preference made %s: %s", preference, ctx.User.Name) + ctx.Redirect(setting.AppSubURL + "/user/settings/account") + return + } if ctx.HasError() { loadAccountData(ctx) diff --git a/templates/user/settings/account.tmpl b/templates/user/settings/account.tmpl index 778103e44..dcb5770ac 100644 --- a/templates/user/settings/account.tmpl +++ b/templates/user/settings/account.tmpl @@ -43,7 +43,30 @@