stop managing global chats subscription if disabled (#497)

Honour the existing "Sync direct and group messages" setting, skipping the attempt to register a global chats subscription accordingly.
mattermost · Feb 14, 2024 · 4e3aa66 · 4e3aa66
1 parent 3ffe251
commit 4e3aa66
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 28 deletions.
diff --git a/server/monitor/monitor.go b/server/monitor/monitor.go
@@ -16,27 +16,29 @@ import (
 const monitoringSystemJobName = "monitoring_system"
 
 type Monitor struct {
-	client           msteams.Client
-	store            store.Store
-	api              plugin.API
-	metrics          metrics.Metrics
-	job              *cluster.Job
-	baseURL          string
-	webhookSecret    string
-	certificate      string
-	useEvaluationAPI bool
+	client             msteams.Client
+	store              store.Store
+	api                plugin.API
+	metrics            metrics.Metrics
+	job                *cluster.Job
+	baseURL            string
+	webhookSecret      string
+	certificate        string
+	useEvaluationAPI   bool
+	syncDirectMessages bool
 }
 
-func New(client msteams.Client, store store.Store, api plugin.API, metrics metrics.Metrics, baseURL string, webhookSecret string, useEvaluationAPI bool, certificate string) *Monitor {
+func New(client msteams.Client, store store.Store, api plugin.API, metrics metrics.Metrics, baseURL string, webhookSecret string, useEvaluationAPI bool, certificate string, syncDirectMessages bool) *Monitor {
 	return &Monitor{
-		client:           client,
-		store:            store,
-		api:              api,
-		metrics:          metrics,
-		baseURL:          baseURL,
-		webhookSecret:    webhookSecret,
-		useEvaluationAPI: useEvaluationAPI,
-		certificate:      certificate,
+		client:             client,
+		store:              store,
+		api:                api,
+		metrics:            metrics,
+		baseURL:            baseURL,
+		webhookSecret:      webhookSecret,
+		useEvaluationAPI:   useEvaluationAPI,
+		certificate:        certificate,
+		syncDirectMessages: syncDirectMessages,
 	}
 }
 
@@ -95,7 +97,7 @@ func (m *Monitor) RunMonitoringSystemJob() {
 		m.checkChannelsSubscriptions(msteamsSubscriptionsMap)
 	}()
 
-	m.checkGlobalSubscriptions(msteamsSubscriptionsMap, allChatsSubscription)
+	m.checkGlobalChatsSubscription(msteamsSubscriptionsMap, allChatsSubscription)
 
 	wg.Wait()
 }
diff --git a/server/monitor/subscriptions.go b/server/monitor/subscriptions.go
@@ -110,13 +110,33 @@ func (m *Monitor) checkChannelsSubscriptions(msteamsSubscriptionsMap map[string]
 // 	}
 // }
 
-func (m *Monitor) checkGlobalSubscriptions(msteamsSubscriptionsMap map[string]*clientmodels.Subscription, allChatsSubscription *clientmodels.Subscription) {
+func (m *Monitor) checkGlobalChatsSubscription(msteamsSubscriptionsMap map[string]*clientmodels.Subscription, allChatsSubscription *clientmodels.Subscription) {
 	subscriptions, err := m.store.ListGlobalSubscriptions()
 	if err != nil {
 		m.api.LogWarn("Unable to get the chat subscriptions from store", "error", err.Error())
 		return
 	}
 
+	// Clean up if we're not syncing direct messages.
+	if !m.syncDirectMessages {
+		// Delete any MS Teams subscription for the global chats, if present.
+		if allChatsSubscription != nil {
+			if err := m.client.DeleteSubscription(allChatsSubscription.ID); err != nil {
+				m.api.LogWarn("Failed to delete old global chats subscriptions", "error", err.Error())
+			}
+		}
+
+		// Delete any global subscriptions (assume at most one here).
+		if len(subscriptions) > 0 {
+			if err := m.store.DeleteSubscription(subscriptions[0].SubscriptionID); err != nil {
+				m.api.LogWarn("Unable to delete the old all chats subscription", "error", err.Error())
+			}
+		}
+
+		return
+	}
+
+	// Create or save a global subscription if we have none.
 	if len(subscriptions) == 0 {
 		if allChatsSubscription == nil {
 			m.CreateAndSaveChatSubscription(nil)
@@ -129,7 +149,10 @@ func (m *Monitor) checkGlobalSubscriptions(msteamsSubscriptionsMap map[string]*c
 		return
 	}
 
+	// We only support one global subscription right now, and it's assumed to be the global
+	// chats subscription.
 	mmSubscription := subscriptions[0]
+
 	// Check if all chats subscription is not present on MS Teams
 	if _, msteamsSubscriptionFound := msteamsSubscriptionsMap[mmSubscription.SubscriptionID]; !msteamsSubscriptionFound {
 		// Create all chats subscription on MS Teams

diff --git a/server/monitor/subscriptions_test.go b/server/monitor/subscriptions_test.go
@@ -127,14 +127,14 @@ func TestMonitorCheckGlobalSubscriptions(t *testing.T) {
 			mockAPI := &plugintest.API{}
 			client := mocksClient.NewClient(t)
 			mockmetrics := mocksMetrics.NewMetrics(t)
-			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "")
+			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "", true)
 			testCase.setupClient(client)
 			testCase.setupAPI(mockAPI)
 			testutils.MockLogs(mockAPI)
 			testCase.setupStore(store)
 			testCase.setupMetrics(mockmetrics)
 
-			monitor.checkGlobalSubscriptions(testCase.msteamsSubscriptionMap, testCase.allChatsSubscription)
+			monitor.checkGlobalChatsSubscription(testCase.msteamsSubscriptionMap, testCase.allChatsSubscription)
 			store.AssertExpectations(t)
 			mockAPI.AssertExpectations(t)
 			client.AssertExpectations(t)
@@ -272,7 +272,7 @@ func TestMonitorCheckChannelSubscriptions(t *testing.T) {
 			mockAPI := &plugintest.API{}
 			client := mocksClient.NewClient(t)
 			mockmetrics := mocksMetrics.NewMetrics(t)
-			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "")
+			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "", true)
 			testCase.setupClient(client)
 			testCase.setupAPI(mockAPI)
 			testutils.MockLogs(mockAPI)
@@ -452,7 +452,7 @@ func TestMonitorRecreateGlobalSubscription(t *testing.T) {
 			mockAPI := &plugintest.API{}
 			client := mocksClient.NewClient(t)
 			mockmetrics := mocksMetrics.NewMetrics(t)
-			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "")
+			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "", true)
 			testCase.setupClient(client)
 			testCase.setupAPI(mockAPI)
 			testutils.MockLogs(mockAPI)
@@ -565,7 +565,7 @@ func TestRecreateChannelSubscription(t *testing.T) {
 			mockAPI := &plugintest.API{}
 			client := mocksClient.NewClient(t)
 			mockmetrics := mocksMetrics.NewMetrics(t)
-			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "")
+			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "", true)
 			testCase.setupClient(client)
 			testCase.setupAPI(mockAPI)
 			testutils.MockLogs(mockAPI)
@@ -656,7 +656,7 @@ func TestMonitorRecreateChatSubscription(t *testing.T) {
 			mockAPI := &plugintest.API{}
 			client := mocksClient.NewClient(t)
 			mockmetrics := mocksMetrics.NewMetrics(t)
-			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "")
+			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "", true)
 			testCase.setupClient(client)
 			testCase.setupAPI(mockAPI)
 			testutils.MockLogs(mockAPI)
@@ -733,7 +733,7 @@ func TestMonitorRefreshSubscription(t *testing.T) {
 			mockAPI := &plugintest.API{}
 			client := mocksClient.NewClient(t)
 			mockmetrics := mocksMetrics.NewMetrics(t)
-			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "")
+			monitor := New(client, store, mockAPI, mockmetrics, "base-url", "webhook-secret", false, "", true)
 			testCase.setupClient(client)
 			testCase.setupAPI(mockAPI)
 			testutils.MockLogs(mockAPI)

diff --git a/server/plugin.go b/server/plugin.go
@@ -280,7 +280,7 @@ func (p *Plugin) start(isRestart bool) {
 		return
 	}
 
-	p.monitor = monitor.New(p.GetClientForApp(), p.store, p.API, p.GetMetrics(), p.GetURL()+"/", p.getConfiguration().WebhookSecret, p.getConfiguration().EvaluationAPI, p.getBase64Certificate())
+	p.monitor = monitor.New(p.GetClientForApp(), p.store, p.API, p.GetMetrics(), p.GetURL()+"/", p.getConfiguration().WebhookSecret, p.getConfiguration().EvaluationAPI, p.getBase64Certificate(), p.GetSyncDirectMessages())
 	if err = p.monitor.Start(); err != nil {
 		p.API.LogError("Unable to start the monitoring system", "error", err.Error())
 	}