PLT-7177: Change the default email frequency to 15 minutes if batching is enabled on the server. (#7036)

* PLT-7177: Found default preference that needs to be changed

* PLT-7177: Front end behaves as desired

* PLT-7177: Changed default batching interval on server side

* PLT-7177: Added unit tests for new default interval

* PLT-7177: Removed unused import

* PLT-7177: Renamed constants to increase clarity

app/email_batching.go

@@ -140,13 +140,14 @@ func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler fu
 		// get how long we need to wait to send notifications to the user
 		var interval int64
 		if result := <-pchan; result.Err != nil {
-			// default to 30 seconds to match the send "immediate" setting
-			interval, _ = strconv.ParseInt(model.PREFERENCE_DEFAULT_EMAIL_INTERVAL, 10, 64)
+			// use the default batching interval if an error ocurrs while fetching user preferences
+			interval, _ = strconv.ParseInt(model.PREFERENCE_EMAIL_INTERVAL_BATCHING_SECONDS, 10, 64)
 		} else {
 			preference := result.Data.(model.Preference)
 
 			if value, err := strconv.ParseInt(preference.Value, 10, 64); err != nil {
-				interval, _ = strconv.ParseInt(model.PREFERENCE_DEFAULT_EMAIL_INTERVAL, 10, 64)
+				// // use the default batching interval if an error ocurrs while deserializing user preferences
+				interval, _ = strconv.ParseInt(model.PREFERENCE_EMAIL_INTERVAL_BATCHING_SECONDS, 10, 64)
 			} else {
 				interval = value
 			}

app/email_batching_test.go

@@ -191,3 +191,83 @@ func TestCheckPendingNotifications(t *testing.T) {
 		t.Fatal("timed out waiting for second post notification")
 	}
 }
+
+/**
+ * Ensures that email batch interval defaults to 15 minutes for users that haven't explicitly set this preference
+ */
+func TestCheckPendingNotificationsDefaultInterval(t *testing.T) {
+	Setup()
+	id1 := model.NewId()
+	job := MakeEmailBatchingJob(128)
+
+	// bypasses recent user activity check
+	store.Must(Srv.Store.Status().SaveOrUpdate(&model.Status{
+		UserId:         id1,
+		LastActivityAt: 9999000,
+	}))
+
+	job.pendingNotifications[id1] = []*batchedNotification{
+		{
+			post: &model.Post{
+				UserId:   id1,
+				CreateAt: 10000000,
+			},
+		},
+	}
+
+	// notifications should not be sent 1s after post was created, because default batch interval is 15mins
+	job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {})
+	if job.pendingNotifications[id1] == nil || len(job.pendingNotifications[id1]) != 1 {
+		t.Fatal("shouldn't have sent queued post")
+	}
+
+	// notifications should be sent 901s after post was created, because default batch interval is 15mins
+	job.checkPendingNotifications(time.Unix(10901, 0), func(string, []*batchedNotification) {})
+	if job.pendingNotifications[id1] != nil || len(job.pendingNotifications[id1]) != 0 {
+		t.Fatal("should have sent queued post")
+	}
+}
+
+/**
+ * Ensures that email batch interval defaults to 15 minutes if user preference is invalid
+ */
+func TestCheckPendingNotificationsCantParseInterval(t *testing.T) {
+	Setup()
+	id1 := model.NewId()
+	job := MakeEmailBatchingJob(128)
+
+	// bypasses recent user activity check
+	store.Must(Srv.Store.Status().SaveOrUpdate(&model.Status{
+		UserId:         id1,
+		LastActivityAt: 9999000,
+	}))
+
+	// preference value is not an integer, so we'll fall back to the default 15min value
+	store.Must(Srv.Store.Preference().Save(&model.Preferences{{
+		UserId:   id1,
+		Category: model.PREFERENCE_CATEGORY_NOTIFICATIONS,
+		Name:     model.PREFERENCE_NAME_EMAIL_INTERVAL,
+		Value:    "notAnIntegerValue",
+	}}))
+
+	job.pendingNotifications[id1] = []*batchedNotification{
+		{
+			post: &model.Post{
+				UserId:   id1,
+				CreateAt: 10000000,
+			},
+		},
+	}
+
+	// notifications should not be sent 1s after post was created, because default batch interval is 15mins
+	job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {})
+	if job.pendingNotifications[id1] == nil || len(job.pendingNotifications[id1]) != 1 {
+		t.Fatal("shouldn't have sent queued post")
+	}
+
+	// notifications should be sent 901s after post was created, because default batch interval is 15mins
+	job.checkPendingNotifications(time.Unix(10901, 0), func(string, []*batchedNotification) {})
+	if job.pendingNotifications[id1] != nil || len(job.pendingNotifications[id1]) != 0 {
+		t.Fatal("should have sent queued post")
+	}
+}

app/notification.go

@@ -332,13 +332,12 @@ func sendNotificationEmail(post *model.Post, user *model.User, channel *model.Ch
 	}
 	if *utils.Cfg.EmailSettings.EnableEmailBatching {
 		var sendBatched bool
-
 		if result := <-Srv.Store.Preference().Get(user.Id, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL); result.Err != nil {
-			// if the call fails, assume it hasn't been set and use the default
+			// if the call fails, assume it hasn't been set and don't batch notifications for this user
 			sendBatched = false
 		} else {
-			// default to not using batching if the setting is set to immediate
-			sendBatched = result.Data.(model.Preference).Value != model.PREFERENCE_DEFAULT_EMAIL_INTERVAL
+			// if the user has chosen to receive notifications immediately, don't batch them
+			sendBatched = result.Data.(model.Preference).Value != model.PREFERENCE_EMAIL_INTERVAL_NO_BATCHING_SECONDS
 		}
 
 		if sendBatched {

model/preference.go

@@ -33,7 +33,9 @@ const (
 
 	PREFERENCE_CATEGORY_NOTIFICATIONS = "notifications"
 	PREFERENCE_NAME_EMAIL_INTERVAL    = "email_interval"
-	PREFERENCE_DEFAULT_EMAIL_INTERVAL = "30" // default to match the interval of the "immediate" setting (ie 30 seconds)
+
+	PREFERENCE_EMAIL_INTERVAL_NO_BATCHING_SECONDS = "30"  // the "immediate" setting is actually 30s
+	PREFERENCE_EMAIL_INTERVAL_BATCHING_SECONDS    = "900" // fifteen minutes is 900 seconds
 )
 
 type Preference struct {

webapp/components/admin_console/email_settings.jsx

@@ -21,9 +21,7 @@ export default class EmailSettings extends AdminSettings {
         super(props);
 
         this.getConfigFromState = this.getConfigFromState.bind(this);
-
         this.handleSaved = this.handleSaved.bind(this);
-
         this.renderSettings = this.renderSettings.bind(this);
     }
 
@@ -138,11 +136,11 @@ export default class EmailSettings extends AdminSettings {
                         <FormattedHTMLMessage
                             key='admin.email.enableEmailBatchingDesc'
                             id='admin.email.enableEmailBatchingDesc'
-                            defaultMessage='When true, users can have email notifications for multiple direct messages and mentions combined into a single email, configurable in <b>Account Settings > Notifications</b>.'
+                            defaultMessage='When true, users will have email notifications for multiple direct messages and mentions combined into a single email. Batching will occur at a default interval of 15 minutes, configurable in Account Settings > Notifications.'
                         />,
                         enableEmailBatchingDisabledText
                     ]}
-                    value={this.state.enableEmailBatching && !this.props.config.ClusterSettings.Enable && this.props.config.ServiceSettings.SiteURL}
+                    value={this.state.enableEmailBatching && !this.props.config.ClusterSettings.Enable && Boolean(this.props.config.ServiceSettings.SiteURL)}
                     onChange={this.handleChange}
                     disabled={!this.state.sendEmailNotifications || this.props.config.ClusterSettings.Enable || !this.props.config.ServiceSettings.SiteURL}
                 />

webapp/components/user_settings/email_notification_setting.jsx

@@ -28,13 +28,20 @@ export default class EmailNotificationSetting extends React.Component {
         super(props);
 
         this.submit = this.submit.bind(this);
-
         this.expand = this.expand.bind(this);
         this.collapse = this.collapse.bind(this);
 
-        this.state = {
-            emailInterval: PreferenceStore.getInt(Preferences.CATEGORY_NOTIFICATIONS, Preferences.EMAIL_INTERVAL, Preferences.INTERVAL_IMMEDIATE)
-        };
+        if (global.mm_config.EnableEmailBatching === 'true') {
+            // when email batching is enabled, the default interval is 15 minutes
+            this.state = {
+                emailInterval: PreferenceStore.getInt(Preferences.CATEGORY_NOTIFICATIONS, Preferences.EMAIL_INTERVAL, Preferences.INTERVAL_FIFTEEN_MINUTES)
+            };
+        } else {
+            // otherwise, the default interval is immediately
+            this.state = {
+                emailInterval: PreferenceStore.getInt(Preferences.CATEGORY_NOTIFICATIONS, Preferences.EMAIL_INTERVAL, Preferences.INTERVAL_IMMEDIATE)
+            };
+        }
     }
 
     handleChange(enableEmail, emailInterval) {

webapp/i18n/en.json

@@ -267,7 +267,7 @@
   "admin.email.emailSuccess": "No errors were reported while sending an email.  Please check your inbox to make sure.",
   "admin.email.enableEmailBatching.clusterEnabled": "Email batching cannot be enabled when High Availability mode is enabled.",
   "admin.email.enableEmailBatching.siteURL": "Email batching cannot be enabled unless the SiteURL is configured in <b>Configuration > SiteURL</b>.",
-  "admin.email.enableEmailBatchingDesc": "When true, users can have email notifications for multiple direct messages and mentions combined into a single email, configurable in <b>Account Settings > Notifications</b>.",
+  "admin.email.enableEmailBatchingDesc": "When true, users will have email notifications for multiple direct messages and mentions combined into a single email. Batching will occur at a default interval of 15 minutes, configurable in Account Settings > Notifications.",
   "admin.email.enableEmailBatchingTitle": "Enable Email Batching:",
   "admin.email.fullPushNotification": "Send full message snippet",
   "admin.email.genericPushNotification": "Send generic description with user and channel names",