MM-60005: dont notify if present (#727)

* make generate cleanup * notify only if unavailable in Teams * log when skipping since notifications disabled * log notifications discard reason * namespace=~ to allow for local use * rm duplicate errors on failing to list subscriptions * add grahps for discarded notifications * missing namespace qualifier on Grafana panel * log tweaks
mattermost · Aug 7, 2024 · 73adb40 · 73adb40
1 parent 7bed450
commit 73adb40
Show file tree

Hide file tree

Showing 19 changed files with 703 additions and 252 deletions.
diff --git a/server/bot_messages.go b/server/bot_messages.go
@@ -77,7 +77,7 @@ func (p *Plugin) SendWelcomeMessageWithNotificationAction(userID string) error {
 func (p *Plugin) makeWelcomeMessageWithNotificationActionPost() *model.Post {
 	msg := []string{
 		"**Notifications from chats and group chats**",
-		"When you enable this feature, you'll be notified here in Mattermost by the MS Teams bot whenever you receive a message from a chat or group chat from Microsoft Teams!",
+		"When you enable this feature, you'll be notified here in Mattermost whenever you're away from Microsoft Teams and receive a message from a chat or group chat.",
 		fmt.Sprintf("![enable notifications picture](%s/static/enable_notifications.gif)", p.GetRelativeURL()),
 	}
 

diff --git a/server/credentials.go b/server/credentials.go
@@ -87,6 +87,13 @@ func getExpectedPermissions() []expectedPermission {
 				Type: "Role",
 			},
 		},
+		{
+			Name: "https://graph.microsoft.com/Presence.Read.All",
+			ResourceAccess: clientmodels.ResourceAccess{
+				ID:   "a70e0c2d-e793-494c-94c4-118fa0a67f42",
+				Type: "Role",
+			},
+		},
 	}
 }
 

diff --git a/server/handlers.go b/server/handlers.go
@@ -143,6 +143,7 @@ func (ah *ActivityHandler) Handle(activity msteams.Activity) error {
 
 func (ah *ActivityHandler) HandleLifecycleEvent(event msteams.Activity) {
 	if event.LifecycleEvent != "reauthorizationRequired" {
+		ah.plugin.GetAPI().LogWarn("Ignoring unknown lifecycle event", "lifecycle_event", event.LifecycleEvent)
 		ah.plugin.GetMetrics().ObserveLifecycleEvent(event.LifecycleEvent, metrics.DiscardedReasonUnknownLifecycleEvent)
 		return
 	}
@@ -156,15 +157,15 @@ func (ah *ActivityHandler) HandleLifecycleEvent(event msteams.Activity) {
 		ah.plugin.GetAPI().LogWarn("Failed to lookup subscription, refreshing anyway", "subscription_id", event.SubscriptionID, "error", err.Error())
 	}
 
-	ah.plugin.GetAPI().LogWarn("Refreshing subscription", "subscription_id", event.SubscriptionID)
+	ah.plugin.GetAPI().LogInfo("Refreshing subscription", "subscription_id", event.SubscriptionID)
 	expiresOn, err := ah.plugin.GetClientForApp().RefreshSubscription(event.SubscriptionID)
 	if err != nil {
 		ah.plugin.GetAPI().LogWarn("Unable to refresh the subscription", "subscription_id", event.SubscriptionID, "error", err.Error())
 		ah.plugin.GetMetrics().ObserveLifecycleEvent(event.LifecycleEvent, metrics.DiscardedReasonFailedToRefresh)
 		return
 	}
 
-	ah.plugin.GetAPI().LogWarn("Refreshed subscription", "subscription_id", event.SubscriptionID, "expires_on", expiresOn.Format("2006-01-02 15:04:05.000 Z07:00"))
+	ah.plugin.GetAPI().LogInfo("Refreshed subscription", "subscription_id", event.SubscriptionID, "expires_on", expiresOn.Format("2006-01-02 15:04:05.000 Z07:00"))
 	ah.plugin.GetMetrics().ObserveSubscription(metrics.SubscriptionRefreshed)
 
 	if err = ah.plugin.GetStore().UpdateSubscriptionExpiresOn(event.SubscriptionID, *expiresOn); err != nil {

diff --git a/server/handlers_test.go b/server/handlers_test.go
@@ -133,6 +133,7 @@ func TestHandleCreatedActivity(t *testing.T) {
 	t.Run("notifications", func(t *testing.T) {
 		type parameters struct {
 			NotificationPref bool
+			OnlineInTeams    bool
 		}
 		runPermutations(t, parameters{}, func(t *testing.T, params parameters) {
 			th.Reset(t)
@@ -162,10 +163,25 @@ func TestHandleCreatedActivity(t *testing.T) {
 				mockTeams.registerChat(activityIds.ChatID, []*model.User{user1, senderUser})
 				mockTeams.registerChatMessage(activityIds.ChatID, activityIds.MessageID, senderUser, "message")
 
+				user1Presence := clientmodels.Presence{
+					UserID: "t" + user1.Id,
+				}
+				if params.OnlineInTeams {
+					user1Presence.Activity = PresenceActivityAvailable
+					user1Presence.Availability = PresenceAvailabilityAvailable
+				} else {
+					user1Presence.Activity = PresenceActivityOffline
+					user1Presence.Availability = PresenceAvailabilityOffline
+				}
+
+				th.appClientMock.On("GetPresencesForUsers", []string{"t" + user1.Id}).Return(map[string]*clientmodels.Presence{
+					"t" + user1.Id: &user1Presence,
+				}, nil).Times(1)
+
 				discardReason := th.p.activityHandler.handleCreatedActivity(activityIds)
 				assert.Equal(t, metrics.DiscardedReasonNone, discardReason)
 
-				if params.NotificationPref {
+				if params.NotificationPref && !params.OnlineInTeams {
 					th.assertDMFromUserRe(t, botUser.Id, user1.Id, "message")
 				} else {
 					th.assertNoDMFromUser(t, botUser.Id, user1.Id, model.GetMillisForTime(time.Now().Add(-5*time.Second)))
@@ -187,6 +203,17 @@ func TestHandleCreatedActivity(t *testing.T) {
 				user2 := th.SetupUser(t, team)
 				th.ConnectUser(t, user2.Id)
 
+				// user2 always prefers to get the notification
+				err = th.p.setNotificationPreference(user2.Id, true)
+				require.NoError(t, err)
+
+				user3 := th.SetupUser(t, team)
+				th.ConnectUser(t, user3.Id)
+
+				// user3 always prefers to get the notification
+				err = th.p.setNotificationPreference(user3.Id, true)
+				require.NoError(t, err)
+
 				botUser, err := th.p.apiClient.User.Get(th.p.botUserID)
 				require.NoError(t, err)
 				th.ConnectUser(t, botUser.Id)
@@ -197,17 +224,41 @@ func TestHandleCreatedActivity(t *testing.T) {
 				}
 
 				mockTeams := newMockTeamsHelper(th)
-				mockTeams.registerGroupChat(activityIds.ChatID, []*model.User{user1, user2, senderUser})
+				mockTeams.registerGroupChat(activityIds.ChatID, []*model.User{user1, user2, user3})
 				mockTeams.registerChatMessage(activityIds.ChatID, activityIds.MessageID, senderUser, "message")
 
+				user1Presence := clientmodels.Presence{
+					UserID: "t" + user1.Id,
+				}
+				if params.OnlineInTeams {
+					user1Presence.Activity = PresenceActivityAvailable
+					user1Presence.Availability = PresenceAvailabilityAvailable
+				} else {
+					user1Presence.Activity = PresenceActivityOffline
+					user1Presence.Availability = PresenceAvailabilityOffline
+				}
+
+				th.appClientMock.On("GetPresencesForUsers", []string{"t" + user1.Id, "t" + user2.Id, "t" + user3.Id}).Return(map[string]*clientmodels.Presence{
+					"t" + user1.Id: &user1Presence,
+					"t" + user2.Id: {
+						UserID:       "t" + user2.Id,
+						Activity:     PresenceActivityOffline,
+						Availability: PresenceAvailabilityOffline,
+					},
+					// no presence for user3: should always get the message
+				}, nil).Times(1)
+
 				discardReason := th.p.activityHandler.handleCreatedActivity(activityIds)
 				assert.Equal(t, metrics.DiscardedReasonNone, discardReason)
 
-				if params.NotificationPref {
+				if params.NotificationPref && !params.OnlineInTeams {
 					th.assertDMFromUserRe(t, botUser.Id, user1.Id, "message")
 				} else {
 					th.assertNoDMFromUser(t, botUser.Id, user1.Id, model.GetMillisForTime(time.Now().Add(-5*time.Second)))
 				}
+
+				th.assertDMFromUserRe(t, botUser.Id, user2.Id, "message")
+				th.assertDMFromUserRe(t, botUser.Id, user3.Id, "message")
 			})
 		})
 	})