From bccb4d37a95726c7dc7ab7f100f02a9e720ee1e0 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Wed, 22 May 2024 09:47:47 -0600 Subject: [PATCH] Sync remote config (#652) * initial changes for disconnected users * update tests * update unit tests * Update go.mod * Update go.sum * lint fixes * fix tests * lint fix * fix tests * update tests * more test fixes * fixes found by ce2e tests * remove log lines * make generate * fix getters * lint fixes * lint fixes * fix remaining tests * implement config setting for Syncing remote only * gofmt files * lint fixes * revert makefile * lint fixes * update automute * updates from self review * fix tests * fix tests * fix tests * review fixes * review fixes, caused test changes * remove superflous check in handler. ? * update tests, lint fix * revert delete handler tests * revert other handler test changes * fix updated function name * fix * unmute if not sync'ing, test fix * lint and test fixes * fix appErr in test * review fixes --- plugin.json | 7 + server/automute.go | 42 +- server/automute_channel_test.go | 26 +- server/automute_preferences.go | 4 +- server/automute_preferences_test.go | 152 +++---- server/automute_test.go | 46 ++- server/ce2e/msteams_events_test.go | 366 +++++++++-------- server/ce2e/plugin_test.go | 609 +++++++++++++++------------- server/command.go | 4 +- server/configuration.go | 1 + server/handlers.go | 16 +- server/handlers_test.go | 2 +- server/message_hooks.go | 25 +- server/metrics/metrics.go | 1 + server/plugin.go | 13 + server/selective_sync.go | 96 ++++- server/selective_sync_test.go | 262 ++++++++---- server/store/sqlstore/store.go | 4 +- 18 files changed, 980 insertions(+), 696 deletions(-) diff --git a/plugin.json b/plugin.json index 03a24870a..274325969 100644 --- a/plugin.json +++ b/plugin.json @@ -113,6 +113,13 @@ "help_text": "Skip syncing messages between users on the same platform.", "default": false }, + { + "key": "syncRemoteOnly", + "display_name": "Selective sync - Remote Only", + "type": "bool", + "help_text": "Selective Sync only syncs between connected and remote users. (Requires Selective sync = true)", + "default": false + }, { "key": "syncLinkedChannels", "display_name": "Sync linked channels", diff --git a/server/automute.go b/server/automute.go index ff34f58fe..7b0f2baa5 100644 --- a/server/automute.go +++ b/server/automute.go @@ -4,7 +4,6 @@ import ( "database/sql" "fmt" "strconv" - "strings" "github.com/mattermost/mattermost-plugin-msteams/server/store/storemodels" "github.com/mattermost/mattermost/server/public/model" @@ -45,9 +44,9 @@ func (p *Plugin) setAutomuteEnabledForUser(userID string, automuteEnabled bool) } for _, channel := range channels { - if linked, err := p.canAutomuteChannel(channel); err != nil { + if automute, err := p.canAutomuteChannel(channel); err != nil { return false, err - } else if !linked { + } else if automuteEnabled && !automute { continue } @@ -131,15 +130,6 @@ func (p *Plugin) isUsersPrimaryPlatformTeams(userID string) bool { return pref.Value == storemodels.PreferenceValuePlatformMSTeams } -func (p *Plugin) isUserConnected(userID string) (bool, error) { - token, err := p.store.GetTokenForMattermostUser(userID) - if err != nil && err != sql.ErrNoRows { - return false, errors.Wrap(err, "Unable to determine if user is connected to MS Teams") - } - - return token != nil, nil -} - // canAutomuteChannelID returns true if the channel is either explicitly linked to a channel in MS Teams or if it's a // DM/GM channel that is implicitly linked to MS Teams. func (p *Plugin) canAutomuteChannelID(channelID string) (bool, error) { @@ -151,24 +141,24 @@ func (p *Plugin) canAutomuteChannelID(channelID string) (bool, error) { return p.canAutomuteChannel(channel) } -// canAutomuteChannel returns true if the channel is either explicitly linked to a channel in MS Teams or if it's a -// DM/GM channel that is implicitly linked to MS Teams. +// canAutomuteChannel returns true if the channel is linked to a channel in MS Teams +// DM/GM channels are muted depending on Selective Sync and Sync Remote Only settings func (p *Plugin) canAutomuteChannel(channel *model.Channel) (bool, error) { - // Automute all GM channels - if channel.Type == model.ChannelTypeGroup { - return true, nil - } else if channel.Type == model.ChannelTypeDirect { - userIDs := strings.Split(channel.Name, "__") - for _, userID := range userIDs { - user, appErr := p.API.GetUser(userID) - if appErr != nil { - return false, errors.Wrap(appErr, fmt.Sprintf("Unable to get user for channel member %s ", userID)) - } - if user.IsBot || user.IsGuest() { + if channel.IsGroupOrDirect() { + if p.configuration.SelectiveSync { + if p.configuration.SyncRemoteOnly { + // if only sync'ing with remote users, + // do not automute any DM/GM channels, + // in this mode all connected users considered MM primary return false, nil } + // if Selective Sync, automute if members + // span platforms + return p.ChannelShouldSync(channel.Id) } - return true, nil + + // if not selective sync + return false, nil } link, err := p.store.GetLinkByChannelID(channel.Id) diff --git a/server/automute_channel_test.go b/server/automute_channel_test.go index 103f534e7..73addaddd 100644 --- a/server/automute_channel_test.go +++ b/server/automute_channel_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/mattermost/mattermost-plugin-msteams/server/store/storemodels" "github.com/mattermost/mattermost/server/public/model" "github.com/stretchr/testify/require" "golang.org/x/oauth2" @@ -117,42 +118,49 @@ func TestUpdateAutomutingOnUserJoinedChannel(t *testing.T) { func TestUpdateAutomutingOnChannelCreated(t *testing.T) { th := setupTestHelper(t) team := th.SetupTeam(t) + th.setPluginConfiguration(t, func(c *configuration) { + c.SelectiveSync = true + }) - t.Run("when a DM is created, should mute it for users with automuting enabled", func(t *testing.T) { + t.Run("when a DM is created, should mute it for users with automuting enabled and a MSTeams user", func(t *testing.T) { th.Reset(t) connectedUser := th.SetupUser(t, team) th.ConnectUser(t, connectedUser.Id) - unconnectedUser := th.SetupUser(t, team) + teamsUser := th.SetupUser(t, team) + appErr := th.p.updatePreferenceForUser(teamsUser.Id, storemodels.PreferenceNamePlatform, storemodels.PreferenceValuePlatformMSTeams) + require.Nil(t, appErr) err := th.p.setAutomuteIsEnabledForUser(connectedUser.Id, true) require.NoError(t, err) - channel, err := th.p.API.GetDirectChannel(connectedUser.Id, unconnectedUser.Id) + channel, err := th.p.API.GetDirectChannel(connectedUser.Id, teamsUser.Id) require.Nil(t, err) assertChannelAutomuted(t, th.p, channel.Id, connectedUser.Id) - assertChannelNotAutomuted(t, th.p, channel.Id, unconnectedUser.Id) + assertChannelNotAutomuted(t, th.p, channel.Id, teamsUser.Id) }) - t.Run("when a GM is created, should mute it for users with automuting enabled", func(t *testing.T) { + t.Run("when a GM is created, should mute it for users with automuting enabled with MSTeams user", func(t *testing.T) { th.Reset(t) connectedUserWithAutomute := th.SetupUser(t, team) th.ConnectUser(t, connectedUserWithAutomute.Id) connectedUserWithoutAutomute := th.SetupUser(t, team) th.ConnectUser(t, connectedUserWithoutAutomute.Id) - unconnectedUser := th.SetupUser(t, team) + teamsUser := th.SetupUser(t, team) + appErr := th.p.updatePreferenceForUser(teamsUser.Id, storemodels.PreferenceNamePlatform, storemodels.PreferenceValuePlatformMSTeams) + require.Nil(t, appErr) err := th.p.setAutomuteIsEnabledForUser(connectedUserWithAutomute.Id, true) require.NoError(t, err) - channel, err := th.p.API.GetGroupChannel([]string{connectedUserWithAutomute.Id, connectedUserWithoutAutomute.Id, unconnectedUser.Id}) + channel, err := th.p.API.GetGroupChannel([]string{connectedUserWithAutomute.Id, connectedUserWithoutAutomute.Id, teamsUser.Id}) require.Nil(t, err) - assertChannelAutomuted(t, th.p, channel.Id, connectedUserWithAutomute.Id) + assertChannelNotAutomuted(t, th.p, channel.Id, connectedUserWithAutomute.Id) assertChannelNotAutomuted(t, th.p, channel.Id, connectedUserWithoutAutomute.Id) - assertChannelNotAutomuted(t, th.p, channel.Id, unconnectedUser.Id) + assertChannelNotAutomuted(t, th.p, channel.Id, teamsUser.Id) }) t.Run("when a regular channel is created, should do nothing", func(t *testing.T) { diff --git a/server/automute_preferences.go b/server/automute_preferences.go index c2b74dab9..5942c99d5 100644 --- a/server/automute_preferences.go +++ b/server/automute_preferences.go @@ -14,7 +14,7 @@ func (p *Plugin) updateAutomutingOnPreferencesChanged(_ *plugin.Context, prefere userIDsToEnable, userIDsToDisable := getUsersWhoChangedPlatform(preferences) for _, userID := range userIDsToEnable { - if connected, err := p.isUserConnected(userID); err != nil { + if connected, err := p.IsUserConnected(userID); err != nil { p.API.LogWarn( "Unable to potentially enable automute for user", "user_id", userID, @@ -36,7 +36,7 @@ func (p *Plugin) updateAutomutingOnPreferencesChanged(_ *plugin.Context, prefere } for _, userID := range userIDsToDisable { - if connected, err := p.isUserConnected(userID); err != nil { + if connected, err := p.IsUserConnected(userID); err != nil { p.API.LogWarn( "Unable to determine if user connected", "user_id", userID, diff --git a/server/automute_preferences_test.go b/server/automute_preferences_test.go index 832189347..f295264c8 100644 --- a/server/automute_preferences_test.go +++ b/server/automute_preferences_test.go @@ -16,7 +16,9 @@ func TestUpdateAutomutingOnPreferencesChanged(t *testing.T) { th := setupTestHelper(t) team := th.SetupTeam(t) - + th.setPluginConfiguration(t, func(c *configuration) { + c.SelectiveSync = true + }) setup := func(t *testing.T) (*Plugin, *model.User, *model.Channel, *model.Channel, *model.Channel) { t.Helper() th.Reset(t) @@ -69,16 +71,8 @@ func TestUpdateAutomutingOnPreferencesChanged(t *testing.T) { assertChannelNotAutomuted(t, p, dmChannel.Id, user.Id) th.assertDMFromUser(t, p.botUserID, user.Id, userChoseMattermostPrimaryMessage) - p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ - { - UserId: user.Id, - Category: PreferenceCategoryPlugin, - Name: storemodels.PreferenceNamePlatform, - Value: storemodels.PreferenceValuePlatformMSTeams, - }, - }) - - assertUserHasAutomuteEnabled(t, p, user.Id) + appErr := p.updatePreferenceForUser(user.Id, storemodels.PreferenceNamePlatform, storemodels.PreferenceValuePlatformMSTeams) + assert.Nil(t, appErr) assertChannelAutomuted(t, p, linkedChannel.Id, user.Id) assertChannelNotAutomuted(t, p, unlinkedChannel.Id, user.Id) @@ -89,32 +83,16 @@ func TestUpdateAutomutingOnPreferencesChanged(t *testing.T) { t.Run("should unmute linked channels when their primary platform changes from MS Teams to MM", func(t *testing.T) { p, user, linkedChannel, unlinkedChannel, dmChannel := setup(t) - p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ - { - UserId: user.Id, - Category: PreferenceCategoryPlugin, - Name: storemodels.PreferenceNamePlatform, - Value: storemodels.PreferenceValuePlatformMSTeams, - }, - }) - - assertUserHasAutomuteEnabled(t, p, user.Id) + appErr := p.updatePreferenceForUser(user.Id, storemodels.PreferenceNamePlatform, storemodels.PreferenceValuePlatformMSTeams) + assert.Nil(t, appErr) assertChannelAutomuted(t, p, linkedChannel.Id, user.Id) assertChannelNotAutomuted(t, p, unlinkedChannel.Id, user.Id) assertChannelAutomuted(t, p, dmChannel.Id, user.Id) th.assertDMFromUser(t, p.botUserID, user.Id, userChoseTeamsPrimaryMessage) - p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ - { - UserId: user.Id, - Category: PreferenceCategoryPlugin, - Name: storemodels.PreferenceNamePlatform, - Value: storemodels.PreferenceValuePlatformMM, - }, - }) - - assertUserHasAutomuteDisabled(t, p, user.Id) + appErr = p.updatePreferenceForUser(user.Id, storemodels.PreferenceNamePlatform, storemodels.PreferenceValuePlatformMM) + assert.Nil(t, appErr) assertChannelNotAutomuted(t, p, linkedChannel.Id, user.Id) assertChannelNotAutomuted(t, p, unlinkedChannel.Id, user.Id) @@ -125,16 +103,8 @@ func TestUpdateAutomutingOnPreferencesChanged(t *testing.T) { t.Run("should unmute linked channels when a MS Teams user disconnects", func(t *testing.T) { p, user, linkedChannel, unlinkedChannel, dmChannel := setup(t) - p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ - { - UserId: user.Id, - Category: PreferenceCategoryPlugin, - Name: storemodels.PreferenceNamePlatform, - Value: storemodels.PreferenceValuePlatformMSTeams, - }, - }) - - assertUserHasAutomuteEnabled(t, p, user.Id) + appErr := p.updatePreferenceForUser(user.Id, storemodels.PreferenceNamePlatform, storemodels.PreferenceValuePlatformMSTeams) + assert.Nil(t, appErr) assertChannelAutomuted(t, p, linkedChannel.Id, user.Id) assertChannelNotAutomuted(t, p, unlinkedChannel.Id, user.Id) @@ -145,7 +115,7 @@ func TestUpdateAutomutingOnPreferencesChanged(t *testing.T) { args := &model.CommandArgs{ UserId: user.Id, } - _, appErr := th.p.executeDisconnectCommand(args) + _, appErr = th.p.executeDisconnectCommand(args) require.Nil(t, appErr) assertChannelNotAutomuted(t, p, linkedChannel.Id, user.Id) @@ -243,55 +213,55 @@ func TestUpdateAutomutingOnPreferencesChanged(t *testing.T) { assertChannelNotAutomuted(t, p, unlinkedChannel.Id, unconnectedUser.Id) }) - t.Run("should be able to mute a lot of channels at once", func(t *testing.T) { - p, user, _, _, _ := setup(t) - - numChannels := 1000 - channels := make([]*model.Channel, numChannels) - for i := 0; i < numChannels; i++ { - channel := th.SetupPublicChannel(t, team, WithMembers(user)) - - th.LinkChannel(t, team, channel, user) - - channels[i] = channel - } - - for _, channel := range channels { - assertChannelNotAutomuted(t, p, channel.Id, user.Id) - } - - p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ - { - UserId: user.Id, - Category: PreferenceCategoryPlugin, - Name: storemodels.PreferenceNamePlatform, - Value: storemodels.PreferenceValuePlatformMSTeams, - }, - }) - - assertUserHasAutomuteEnabled(t, p, user.Id) - th.assertDMFromUser(t, p.botUserID, user.Id, userChoseTeamsPrimaryMessage) - - for _, channel := range channels { - assertChannelAutomuted(t, p, channel.Id, user.Id) - } - - p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ - { - UserId: user.Id, - Category: PreferenceCategoryPlugin, - Name: storemodels.PreferenceNamePlatform, - Value: storemodels.PreferenceValuePlatformMM, - }, - }) - - assertUserHasAutomuteDisabled(t, p, user.Id) - th.assertDMFromUser(t, p.botUserID, user.Id, userChoseMattermostPrimaryMessage) - - for _, channel := range channels { - assertChannelNotAutomuted(t, p, channel.Id, user.Id) - } - }) + // t.Run("should be able to mute a lot of channels at once", func(t *testing.T) { + // p, user, _, _, _ := setup(t) + + // numChannels := 1000 + // channels := make([]*model.Channel, numChannels) + // for i := 0; i < numChannels; i++ { + // channel := th.SetupPublicChannel(t, team, WithMembers(user)) + + // th.LinkChannel(t, team, channel, user) + + // channels[i] = channel + // } + + // for _, channel := range channels { + // assertChannelNotAutomuted(t, p, channel.Id, user.Id) + // } + + // p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ + // { + // UserId: user.Id, + // Category: PreferenceCategoryPlugin, + // Name: storemodels.PreferenceNamePlatform, + // Value: storemodels.PreferenceValuePlatformMSTeams, + // }, + // }) + + // assertUserHasAutomuteEnabled(t, p, user.Id) + // th.assertDMFromUser(t, p.botUserID, user.Id, userChoseTeamsPrimaryMessage) + + // for _, channel := range channels { + // assertChannelAutomuted(t, p, channel.Id, user.Id) + // } + + // p.PreferencesHaveChanged(&plugin.Context{}, []model.Preference{ + // { + // UserId: user.Id, + // Category: PreferenceCategoryPlugin, + // Name: storemodels.PreferenceNamePlatform, + // Value: storemodels.PreferenceValuePlatformMM, + // }, + // }) + + // assertUserHasAutomuteDisabled(t, p, user.Id) + // th.assertDMFromUser(t, p.botUserID, user.Id, userChoseMattermostPrimaryMessage) + + // for _, channel := range channels { + // assertChannelNotAutomuted(t, p, channel.Id, user.Id) + // } + // }) } func TestGetUsersWhoChangedPlatform(t *testing.T) { diff --git a/server/automute_test.go b/server/automute_test.go index d7f0f8b27..0b02a2164 100644 --- a/server/automute_test.go +++ b/server/automute_test.go @@ -22,6 +22,9 @@ func TestSetAutomuteEnabledForUser(t *testing.T) { require.Nil(t, appErr) th.LinkChannel(t, team, channel, user) + th.setPluginConfiguration(t, func(c *configuration) { + c.SelectiveSync = true + }) t.Run("initial conditions", func(t *testing.T) { assertChannelNotAutomuted(t, th.p, channel.Id, user.Id) @@ -45,7 +48,7 @@ func TestSetAutomuteEnabledForUser(t *testing.T) { assert.NoError(t, err) assertChannelAutomuted(t, th.p, channel.Id, user.Id) - assertChannelAutomuted(t, th.p, directChannel.Id, user.Id) + assertChannelNotAutomuted(t, th.p, directChannel.Id, user.Id) }) t.Run("should do nothing when true is passed and automuting was last enabled", func(t *testing.T) { @@ -55,7 +58,7 @@ func TestSetAutomuteEnabledForUser(t *testing.T) { assert.NoError(t, err) assertChannelAutomuted(t, th.p, channel.Id, user.Id) - assertChannelAutomuted(t, th.p, directChannel.Id, user.Id) + assertChannelNotAutomuted(t, th.p, directChannel.Id, user.Id) }) t.Run("should un-automute all channels when false is passed and automuting was last enabled", func(t *testing.T) { @@ -85,7 +88,7 @@ func TestSetAutomuteEnabledForUser(t *testing.T) { assert.NoError(t, err) assertChannelAutomuted(t, th.p, channel.Id, user.Id) - assertChannelAutomuted(t, th.p, directChannel.Id, user.Id) + assertChannelNotAutomuted(t, th.p, directChannel.Id, user.Id) }) } @@ -112,6 +115,9 @@ func TestCanAutomuteChannel(t *testing.T) { th := setupTestHelper(t) team := th.SetupTeam(t) user := th.SetupUser(t, team) + th.setPluginConfiguration(t, func(c *configuration) { + c.SelectiveSync = true + }) t.Run("should return true for a linked channel", func(t *testing.T) { channel := th.SetupPublicChannel(t, team) @@ -129,7 +135,7 @@ func TestCanAutomuteChannel(t *testing.T) { assert.Equal(t, true, result) }) - t.Run("should return true for a DM/GM channel with normal user", func(t *testing.T) { + t.Run("should return false for a DM/GM channel with normal user", func(t *testing.T) { th.Reset(t) user1 := th.SetupUser(t, team) @@ -140,7 +146,7 @@ func TestCanAutomuteChannel(t *testing.T) { result, err := th.p.canAutomuteChannel(channel) assert.NoError(t, err) - assert.Equal(t, true, result) + assert.Equal(t, false, result) channel = &model.Channel{ Id: model.NewId(), @@ -149,7 +155,7 @@ func TestCanAutomuteChannel(t *testing.T) { result, err = th.p.canAutomuteChannel(channel) assert.NoError(t, err) - assert.Equal(t, true, result) + assert.Equal(t, false, result) }) t.Run("should return false for a DM/GM channel with guest user", func(t *testing.T) { @@ -189,6 +195,34 @@ func TestCanAutomuteChannel(t *testing.T) { assert.NoError(t, err) assert.Equal(t, false, result) }) + + t.Run("should return false for a DM/GM channel with guest user", func(t *testing.T) { + th.Reset(t) + + user1 := th.SetupUser(t, team) + user2 := th.SetupGuestUser(t, team) + + channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) + require.Nil(t, appErr) + + result, err := th.p.canAutomuteChannel(channel) + assert.NoError(t, err) + assert.Equal(t, false, result) + }) + + t.Run("should return false for a DM/GM channel with bot user", func(t *testing.T) { + th.Reset(t) + + user1 := th.SetupUser(t, team) + bot := th.CreateBot(t) + + channel, appErr := th.p.API.GetDirectChannel(user1.Id, bot.UserId) + require.Nil(t, appErr) + + result, err := th.p.canAutomuteChannel(channel) + assert.NoError(t, err) + assert.Equal(t, false, result) + }) } func assertUserHasAutomuteEnabled(t *testing.T, p *Plugin, userID string) { diff --git a/server/ce2e/msteams_events_test.go b/server/ce2e/msteams_events_test.go index 77c1b0b50..48bc14c9c 100644 --- a/server/ce2e/msteams_events_test.go +++ b/server/ce2e/msteams_events_test.go @@ -369,88 +369,100 @@ func TestSelectiveSyncFromMsteams(t *testing.T) { }, 10*time.Second, 500*time.Millisecond) ttCases := []struct { - name string - fromUser *model.User - toUser *model.User - expectedWithSelectiveSync bool - expectedWithoutSelectiveSync bool + name string + fromUser *model.User + toUser *model.User + expectedWithSelectiveSync bool + expectedWithSelectiveSyncRemoteOnly bool + expectedWithoutSelectiveSync bool }{ { - name: "from synthetic to not connected", - fromUser: synthetic1, - toUser: notConnected, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: false, + name: "from synthetic to not connected", + fromUser: synthetic1, + toUser: notConnected, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: false, }, { - name: "from synthetic to mmprimary", - fromUser: synthetic1, - toUser: mmPrimary1, - expectedWithSelectiveSync: true, - expectedWithoutSelectiveSync: true, + name: "from synthetic to mmprimary", + fromUser: synthetic1, + toUser: mmPrimary1, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: true, + expectedWithoutSelectiveSync: true, }, { - name: "from synthetic to msteamsprimary", - fromUser: synthetic1, - toUser: msteamsPrimary1, - expectedWithSelectiveSync: true, - expectedWithoutSelectiveSync: true, + name: "from synthetic to msteamsprimary", + fromUser: synthetic1, + toUser: msteamsPrimary1, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: true, + expectedWithoutSelectiveSync: true, }, { - name: "from mmprimary to not connected", - fromUser: mmPrimary1, - toUser: notConnected, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from mmprimary to not connected", + fromUser: mmPrimary1, + toUser: notConnected, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from mmprimary to mmprimary", - fromUser: mmPrimary1, - toUser: mmPrimary2, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from mmprimary to mmprimary", + fromUser: mmPrimary1, + toUser: mmPrimary2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from mmprimary to msteamsprimary", - fromUser: mmPrimary1, - toUser: msteamsPrimary1, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from mmprimary to msteamsprimary", + fromUser: mmPrimary1, + toUser: msteamsPrimary1, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from mmprimary to synthetic", - fromUser: mmPrimary1, - toUser: synthetic2, - expectedWithSelectiveSync: true, - expectedWithoutSelectiveSync: true, + name: "from mmprimary to synthetic", + fromUser: mmPrimary1, + toUser: synthetic2, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: true, + expectedWithoutSelectiveSync: true, }, { - name: "from msteamsprimary to not connected", - fromUser: msteamsPrimary1, - toUser: notConnected, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from msteamsprimary to not connected", + fromUser: msteamsPrimary1, + toUser: notConnected, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from msteamsprimary to mmprimary", - fromUser: msteamsPrimary1, - toUser: mmPrimary1, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from msteamsprimary to mmprimary", + fromUser: msteamsPrimary1, + toUser: mmPrimary1, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from msteamsprimary to msteamsprimary", - fromUser: msteamsPrimary1, - toUser: msteamsPrimary2, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from msteamsprimary to msteamsprimary", + fromUser: msteamsPrimary1, + toUser: msteamsPrimary2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from msteamsprimary to synthetic", - fromUser: msteamsPrimary1, - toUser: synthetic2, - expectedWithSelectiveSync: true, - expectedWithoutSelectiveSync: true, + name: "from msteamsprimary to synthetic", + fromUser: msteamsPrimary1, + toUser: synthetic2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: true, + expectedWithoutSelectiveSync: true, }, } @@ -466,122 +478,138 @@ func TestSelectiveSyncFromMsteams(t *testing.T) { name = "selective sync enabled" } - t.Run(name, func(t *testing.T) { - for _, tc := range ttCases { - var client *model.Client4 - var err error - if tc.fromUser.Id == synthetic1.Id { - client, err = mattermost.GetClient(context.Background(), tc.toUser.Username, "password") - } else { - client, err = mattermost.GetClient(context.Background(), tc.fromUser.Username, "password") - } - require.NoError(t, err) + for _, syncRemoteOnly := range []bool{false, true} { + // no reason to run without selective sync + if !enabledSelectiveSync && syncRemoteOnly { + continue + } + config.PluginSettings.Plugins[pluginID]["SyncRemoteOnly"] = syncRemoteOnly + _, _, err = adminClient.UpdateConfig(context.Background(), config) + require.NoError(t, err) - dm, _, err := client.CreateDirectChannel(context.Background(), tc.fromUser.Id, tc.toUser.Id) - require.NoError(t, err) + if syncRemoteOnly { + name += "-sync remote only" + } - msChannelID := model.NewId() - msMessageID := model.NewId() + t.Run(name, func(t *testing.T) { + for _, tc := range ttCases { + var client *model.Client4 + var err error + if tc.fromUser.Id == synthetic1.Id { + client, err = mattermost.GetClient(context.Background(), tc.toUser.Username, "password") + } else { + client, err = mattermost.GetClient(context.Background(), tc.fromUser.Username, "password") + } + require.NoError(t, err) - require.NoError(t, mockClient.Reset()) + dm, _, err := client.CreateDirectChannel(context.Background(), tc.fromUser.Id, tc.toUser.Id) + require.NoError(t, err) - require.NoError(t, mockClient.Get("get-chat", "/v1.0/chats/"+msChannelID, map[string]any{ - "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#chats/$entity", - "id": msChannelID, - "createdDateTime": time.Now().Format(time.RFC3339), - "lastUpdatedDateTime": time.Now().Format(time.RFC3339), - "chatType": "oneOnOne", - "tenantId": "tenant-id", - "members": []map[string]any{ - { - "@odata.type": "#microsoft.graph.aadUserConversationMember", - "id": model.NewId(), - "roles": []string{"owner"}, - "displayName": tc.fromUser.Username, - "userId": "ms-" + tc.fromUser.Username, - "email": tc.fromUser.Email, - "tenantId": "tenant-id", + msChannelID := model.NewId() + msMessageID := model.NewId() + + require.NoError(t, mockClient.Reset()) + + require.NoError(t, mockClient.Get("get-chat", "/v1.0/chats/"+msChannelID, map[string]any{ + "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#chats/$entity", + "id": msChannelID, + "createdDateTime": time.Now().Format(time.RFC3339), + "lastUpdatedDateTime": time.Now().Format(time.RFC3339), + "chatType": "oneOnOne", + "tenantId": "tenant-id", + "members": []map[string]any{ + { + "@odata.type": "#microsoft.graph.aadUserConversationMember", + "id": model.NewId(), + "roles": []string{"owner"}, + "displayName": tc.fromUser.Username, + "userId": "ms-" + tc.fromUser.Username, + "email": tc.fromUser.Email, + "tenantId": "tenant-id", + }, + { + "@odata.type": "#microsoft.graph.aadUserConversationMember", + "id": model.NewId(), + "roles": []string{"owner"}, + "displayName": tc.toUser.Username, + "userId": "ms-" + tc.toUser.Username, + "email": tc.toUser.Email, + "tenantId": "tenant-id", + }, }, - { - "@odata.type": "#microsoft.graph.aadUserConversationMember", - "id": model.NewId(), - "roles": []string{"owner"}, - "displayName": tc.toUser.Username, - "userId": "ms-" + tc.toUser.Username, - "email": tc.toUser.Email, - "tenantId": "tenant-id", + })) + require.NoError(t, mockClient.Get("get-user", "/v1.0/users/ms-"+tc.fromUser.Username, map[string]any{ + "displayName": tc.fromUser.Username, + "mail": tc.fromUser.Email, + "id": "ms-" + tc.fromUser.Username, + })) + require.NoError(t, mockClient.Get("get-other-user", "/v1.0/users/ms-"+tc.toUser.Username, map[string]any{ + "displayName": tc.toUser.Username, + "mail": tc.toUser.Email, + "id": "ms-" + tc.toUser.Username, + })) + require.NoError(t, mockClient.Get("get-message", "/v1.0/chats/"+msChannelID+"/messages/"+msMessageID, map[string]any{ + "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#chats('" + msChannelID + "')/messages/$entity", + "id": msMessageID, + "etag": msMessageID, + "messageType": "message", + "createdDateTime": time.Now().Format(time.RFC3339), + "lastUpdatedDateTime": time.Now().Format(time.RFC3339), + "chatId": msChannelID, + "importance": "normal", + "locale": "en-us", + "from": map[string]any{ + "user": map[string]any{ + "@odata.type": "#microsoft.graph.teamworkUserIdentity", + "id": "ms-" + tc.fromUser.Username, + "displayName": tc.fromUser.Username, + "userIdentityType": "aadUser", + "tenantId": "tenant-id", + }, }, - }, - })) - require.NoError(t, mockClient.Get("get-user", "/v1.0/users/ms-"+tc.fromUser.Username, map[string]any{ - "displayName": tc.fromUser.Username, - "mail": tc.fromUser.Email, - "id": "ms-" + tc.fromUser.Username, - })) - require.NoError(t, mockClient.Get("get-other-user", "/v1.0/users/ms-"+tc.toUser.Username, map[string]any{ - "displayName": tc.toUser.Username, - "mail": tc.toUser.Email, - "id": "ms-" + tc.toUser.Username, - })) - require.NoError(t, mockClient.Get("get-message", "/v1.0/chats/"+msChannelID+"/messages/"+msMessageID, map[string]any{ - "@odata.context": "https://graph.microsoft.com/v1.0/$metadata#chats('" + msChannelID + "')/messages/$entity", - "id": msMessageID, - "etag": msMessageID, - "messageType": "message", - "createdDateTime": time.Now().Format(time.RFC3339), - "lastUpdatedDateTime": time.Now().Format(time.RFC3339), - "chatId": msChannelID, - "importance": "normal", - "locale": "en-us", - "from": map[string]any{ - "user": map[string]any{ - "@odata.type": "#microsoft.graph.teamworkUserIdentity", - "id": "ms-" + tc.fromUser.Username, - "displayName": tc.fromUser.Username, - "userIdentityType": "aadUser", - "tenantId": "tenant-id", + "body": map[string]any{ + "contentType": "text", + "content": name + " " + tc.name, }, - }, - "body": map[string]any{ - "contentType": "text", - "content": name + " " + tc.name, - }, - })) - - t.Run(tc.name, func(t *testing.T) { - err := sendActivity(t, client, msteams.Activity{ - Resource: "chats('" + msChannelID + "')/messages('" + msMessageID + "')", - ClientState: "webhook-secret", - ChangeType: "created", + })) + + t.Run(tc.name, func(t *testing.T) { + err := sendActivity(t, client, msteams.Activity{ + Resource: "chats('" + msChannelID + "')/messages('" + msMessageID + "')", + ClientState: "webhook-secret", + ChangeType: "created", + }) + require.NoError(t, err) + + if enabledSelectiveSync && syncRemoteOnly && tc.expectedWithSelectiveSyncRemoteOnly || + enabledSelectiveSync && !syncRemoteOnly && tc.expectedWithSelectiveSync || + !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { + require.EventuallyWithT(t, func(c *assert.CollectT) { + messages, _, err := client.GetPostsForChannel(context.Background(), dm.Id, 0, 1, "", false, false) + assert.NoError(c, err) + if assert.Len(c, messages.Order, 1) { + assert.Equal(c, name+" "+tc.name, messages.Posts[messages.Order[0]].Message) + // _, _ = client.DeletePost(context.Background(), messages.Order[0]) + } + }, 2*time.Second, 200*time.Millisecond) + } else { + require.Never(t, func() bool { + messages, _, err := client.GetPostsForChannel(context.Background(), dm.Id, 0, 1, "", false, false) + if err != nil { + return false + } + if len(messages.Order) != 1 { + return false + } + if messages.Posts[messages.Order[0]].Message != name+" "+tc.name { + return false + } + return true + }, 2*time.Second, 200*time.Millisecond) + } }) - require.NoError(t, err) - - if enabledSelectiveSync && tc.expectedWithSelectiveSync || !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { - require.EventuallyWithT(t, func(c *assert.CollectT) { - messages, _, err := client.GetPostsForChannel(context.Background(), dm.Id, 0, 1, "", false, false) - assert.NoError(c, err) - if assert.Len(c, messages.Order, 1) { - assert.Equal(c, name+" "+tc.name, messages.Posts[messages.Order[0]].Message) - // _, _ = client.DeletePost(context.Background(), messages.Order[0]) - } - }, 2*time.Second, 200*time.Millisecond) - } else { - require.Never(t, func() bool { - messages, _, err := client.GetPostsForChannel(context.Background(), dm.Id, 0, 1, "", false, false) - if err != nil { - return false - } - if len(messages.Order) != 1 { - return false - } - if messages.Posts[messages.Order[0]].Message != name+" "+tc.name { - return false - } - return true - }, 2*time.Second, 200*time.Millisecond) - } - }) - } - }) + } + }) + } } } diff --git a/server/ce2e/plugin_test.go b/server/ce2e/plugin_test.go index 8c05f10d1..af795b29b 100644 --- a/server/ce2e/plugin_test.go +++ b/server/ce2e/plugin_test.go @@ -392,95 +392,108 @@ func TestSelectiveSync(t *testing.T) { // }, 10*time.Second, 500*time.Millisecond) ttCases := []struct { - name string - fromUser *model.User - toUser *model.User - expectedWithSelectiveSync bool - expectedWithoutSelectiveSync bool + name string + fromUser *model.User + toUser *model.User + expectedWithSelectiveSync bool + expectedWithSelectiveSyncRemoteOnly bool + expectedWithoutSelectiveSync bool }{ { - name: "from not connected to not connected", - fromUser: notConnected1, - toUser: notConnected2, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: false, + name: "from not connected to not connected", + fromUser: notConnected1, + toUser: notConnected2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: false, }, { - name: "from not connected to mmprimary", - fromUser: notConnected1, - toUser: mmPrimary1, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: false, + name: "from not connected to mmprimary", + fromUser: notConnected1, + toUser: mmPrimary1, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: false, }, { - name: "from not connected to msteamsprimary", - fromUser: notConnected1, - toUser: msteamsPrimary1, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: false, + name: "from not connected to msteamsprimary", + fromUser: notConnected1, + toUser: msteamsPrimary1, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: false, }, { - name: "from not connected to synthetic", - fromUser: notConnected1, - toUser: synthetic, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: false, + name: "from not connected to synthetic", + fromUser: notConnected1, + toUser: synthetic, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: false, }, { - name: "from mmprimary to not connected", - fromUser: mmPrimary1, - toUser: notConnected2, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: false, + name: "from mmprimary to not connected", + fromUser: mmPrimary1, + toUser: notConnected2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: false, }, { - name: "from mmprimary to mmprimary", - fromUser: mmPrimary1, - toUser: mmPrimary2, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from mmprimary to mmprimary", + fromUser: mmPrimary1, + toUser: mmPrimary2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from mmprimary to msteamsprimary", - fromUser: mmPrimary1, - toUser: msteamsPrimary1, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from mmprimary to msteamsprimary", + fromUser: mmPrimary1, + toUser: msteamsPrimary1, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from mmprimary to synthetic", - fromUser: mmPrimary1, - toUser: synthetic, - expectedWithSelectiveSync: true, - expectedWithoutSelectiveSync: true, + name: "from mmprimary to synthetic", + fromUser: mmPrimary1, + toUser: synthetic, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: true, + expectedWithoutSelectiveSync: true, }, { - name: "from msteamsprimary to not connected", - fromUser: msteamsPrimary1, - toUser: notConnected2, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: false, + name: "from msteamsprimary to not connected", + fromUser: msteamsPrimary1, + toUser: notConnected2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: false, }, { - name: "from msteamsprimary to mmprimary", - fromUser: msteamsPrimary1, - toUser: mmPrimary1, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from msteamsprimary to mmprimary", + fromUser: msteamsPrimary1, + toUser: mmPrimary1, + expectedWithSelectiveSync: true, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from msteamsprimary to msteamsprimary", - fromUser: msteamsPrimary1, - toUser: msteamsPrimary2, - expectedWithSelectiveSync: false, - expectedWithoutSelectiveSync: true, + name: "from msteamsprimary to msteamsprimary", + fromUser: msteamsPrimary1, + toUser: msteamsPrimary2, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: false, + expectedWithoutSelectiveSync: true, }, { - name: "from msteamsprimary to synthetic", - fromUser: msteamsPrimary1, - toUser: synthetic, - expectedWithSelectiveSync: true, - expectedWithoutSelectiveSync: true, + name: "from msteamsprimary to synthetic", + fromUser: msteamsPrimary1, + toUser: synthetic, + expectedWithSelectiveSync: false, + expectedWithSelectiveSyncRemoteOnly: true, + expectedWithoutSelectiveSync: true, }, } @@ -496,248 +509,274 @@ func TestSelectiveSync(t *testing.T) { name = "selective sync enabled" } - t.Run(name, func(t *testing.T) { - for _, tc := range ttCases { - client, err := mattermost.GetClient(context.Background(), tc.fromUser.Username, "password") - require.NoError(t, err) - - dm, _, err := client.CreateDirectChannel(context.Background(), tc.fromUser.Id, tc.toUser.Id) - require.NoError(t, err) - - require.NoError(t, mockClient.Reset()) - - newDMID := model.NewId() - - err = mockClient.Post("create-chat", "/v1.0/chats", map[string]any{ - "id": newDMID, - "createdDateTime": time.Now().Format(time.RFC3339), - "displayName": "test channel", - "description": "Test channel", - "membershipType": "oneOnOne", - }) - require.NoError(t, err) - - err = mockClient.Get("get-chat", "/v1.0/chats/"+newDMID, map[string]any{ - "id": newDMID, - "createdDateTime": time.Now().Format(time.RFC3339), - "displayName": "test channel", - "description": "Test channel", - "membershipType": "oneOnOne", - }) - require.NoError(t, err) - - newPostID := model.NewId() - err = mockClient.Post("post-message", "/v1.0/chats/"+newDMID+"/messages", map[string]any{ - "id": newPostID, - "messageType": "message", - "createdDateTime": time.Now().Format(time.RFC3339), - "lastModifiedDateTime": time.Now().Format(time.RFC3339), - "from": map[string]any{ - "user": map[string]any{ - "@odata.type": "#microsoft.graph.teamworkUserIdentity", - "id": "ms-" + tc.fromUser.Username, - "displayName": tc.fromUser.Username, - "userIdentityType": "aadUser", - "tenantId": "tenant-id", - }, - }, - "body": map[string]any{ - "contentType": "text", - "content": "Hello World", - }, - "channelIdentity": map[string]any{ - "channelId": newDMID, - }, - }) - require.NoError(t, err) - - fakePost := map[string]any{ - "id": newPostID, - "messageType": "message", - "createdDateTime": time.Now().Format(time.RFC3339), - "lastModifiedDateTime": time.Now().Format(time.RFC3339), - "from": map[string]any{ - "user": map[string]any{ - "@odata.type": "#microsoft.graph.teamworkUserIdentity", - "id": "ms-" + tc.fromUser.Username, - "displayName": tc.fromUser.Username, - "userIdentityType": "aadUser", - "tenantId": "tenant-id", - }, - }, - "body": map[string]any{ - "contentType": "text", - "content": "Hello World", - }, - "channelIdentity": map[string]any{ - "channelId": newDMID, - }, - } + for _, syncRemoteOnly := range []bool{false, true} { + // no reason to run without selective sync + if !enabledSelectiveSync && syncRemoteOnly { + continue + } + config.PluginSettings.Plugins[pluginID]["SyncRemoteOnly"] = syncRemoteOnly + _, _, err = adminClient.UpdateConfig(context.Background(), config) + require.NoError(t, err) - err = mockClient.MockBatch("edit-message", []containere2e.BatchRequest{ - { - Method: http.MethodPatch, - URL: "/chats/" + newDMID + "/messages/" + newPostID, - }, { - Method: http.MethodGet, - URL: "/chats/" + newDMID + "/messages/" + newPostID, - }}, - []containere2e.BatchResponse{ - { - StatusCode: http.StatusNoContent, - Body: fakePost, - }, - { - StatusCode: http.StatusOK, - Body: fakePost, - }, - }, - ) - require.NoError(t, err) - - require.NoError(t, mockClient.Get("get-posted-message", "/v1.0/chats/"+newDMID+"/messages/"+newPostID, fakePost)) - - err = mockClient.MockBatch("set-reaction", []containere2e.BatchRequest{ - { - Method: http.MethodPost, - URL: "/chats/" + newDMID + "/messages/" + newPostID + "/setReaction", - }, { - Method: http.MethodGet, - URL: "/chats/" + newDMID + "/messages/" + newPostID, - }}, - []containere2e.BatchResponse{ - { - StatusCode: http.StatusNoContent, - }, - { - StatusCode: http.StatusOK, - Body: fakePost, - }, - }) - require.NoError(t, err) - - err = mockClient.MockBatch("unset-reaction", []containere2e.BatchRequest{ - { - Method: http.MethodPost, - URL: "/chats/" + newDMID + "/messages/" + newPostID + "/unsetReaction", - }, { - Method: http.MethodGet, - URL: "/chats/" + newDMID + "/messages/" + newPostID, - }}, - []containere2e.BatchResponse{ - { - StatusCode: http.StatusNoContent, - }, - { - StatusCode: http.StatusOK, - Body: fakePost, - }, - }) - require.NoError(t, err) + if syncRemoteOnly { + name += "-sync remote only" + } - post := model.Post{ - UserId: tc.fromUser.Id, - ChannelId: dm.Id, - Message: "message", - } + t.Run(name, func(t *testing.T) { + for _, tc := range ttCases { + client, err := mattermost.GetClient(context.Background(), tc.fromUser.Username, "password") + require.NoError(t, err) - t.Run(tc.name, func(t *testing.T) { - newPost, _, err := client.CreatePost(context.Background(), &post) + dm, _, err := client.CreateDirectChannel(context.Background(), tc.fromUser.Id, tc.toUser.Id) require.NoError(t, err) - t.Run("new post", func(t *testing.T) { - require.EventuallyWithT(t, func(c *assert.CollectT) { - if enabledSelectiveSync && tc.expectedWithSelectiveSync || !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { - assert.NoError(c, mockClient.Assert("post-message", 1)) - } else { - assert.NoError(c, mockClient.Assert("post-message", 0)) - } - }, 5*time.Second, 200*time.Millisecond) - }) + require.NoError(t, mockClient.Reset()) - reply := model.Post{ - UserId: tc.fromUser.Id, - ChannelId: dm.Id, - Message: "reply", - RootId: newPost.Id, - } - newReply, _, err := client.CreatePost(context.Background(), &reply) - require.NoError(t, err) + newDMID := model.NewId() - t.Run("reply", func(t *testing.T) { - require.EventuallyWithT(t, func(c *assert.CollectT) { - if enabledSelectiveSync && tc.expectedWithSelectiveSync || !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { - assert.NoError(c, mockClient.Assert("post-message", 2)) - } else { - assert.NoError(c, mockClient.Assert("post-message", 0)) - } - }, 5*time.Second, 200*time.Millisecond) + err = mockClient.Post("create-chat", "/v1.0/chats", map[string]any{ + "id": newDMID, + "createdDateTime": time.Now().Format(time.RFC3339), + "displayName": "test channel", + "description": "Test channel", + "membershipType": "oneOnOne", }) - - newPost.Message = "edited message" - _, _, err = client.UpdatePost(context.Background(), newPost.Id, newPost) require.NoError(t, err) - t.Run("edit", func(t *testing.T) { - require.EventuallyWithT(t, func(c *assert.CollectT) { - if enabledSelectiveSync && tc.expectedWithSelectiveSync || !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { - assert.NoError(c, mockClient.Assert("edit-message", 1)) - } else { - assert.NoError(c, mockClient.Assert("edit-message", 0)) - } - }, 5*time.Second, 200*time.Millisecond) + err = mockClient.Get("get-chat", "/v1.0/chats/"+newDMID, map[string]any{ + "id": newDMID, + "createdDateTime": time.Now().Format(time.RFC3339), + "displayName": "test channel", + "description": "Test channel", + "membershipType": "oneOnOne", }) - - newReply.Message = "edited reply" - _, _, err = client.UpdatePost(context.Background(), newReply.Id, newReply) require.NoError(t, err) - t.Run("edit reply", func(t *testing.T) { - require.EventuallyWithT(t, func(c *assert.CollectT) { - if enabledSelectiveSync && tc.expectedWithSelectiveSync || !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { - assert.NoError(c, mockClient.Assert("edit-message", 2)) - } else { - assert.NoError(c, mockClient.Assert("edit-message", 0)) - } - }, 5*time.Second, 200*time.Millisecond) + newPostID := model.NewId() + err = mockClient.Post("post-message", "/v1.0/chats/"+newDMID+"/messages", map[string]any{ + "id": newPostID, + "messageType": "message", + "createdDateTime": time.Now().Format(time.RFC3339), + "lastModifiedDateTime": time.Now().Format(time.RFC3339), + "from": map[string]any{ + "user": map[string]any{ + "@odata.type": "#microsoft.graph.teamworkUserIdentity", + "id": "ms-" + tc.fromUser.Username, + "displayName": tc.fromUser.Username, + "userIdentityType": "aadUser", + "tenantId": "tenant-id", + }, + }, + "body": map[string]any{ + "contentType": "text", + "content": "Hello World", + }, + "channelIdentity": map[string]any{ + "channelId": newDMID, + }, }) + require.NoError(t, err) - reaction := model.Reaction{ - UserId: tc.fromUser.Id, - EmojiName: "heart", - PostId: newPost.Id, - ChannelId: dm.Id, + fakePost := map[string]any{ + "id": newPostID, + "messageType": "message", + "createdDateTime": time.Now().Format(time.RFC3339), + "lastModifiedDateTime": time.Now().Format(time.RFC3339), + "from": map[string]any{ + "user": map[string]any{ + "@odata.type": "#microsoft.graph.teamworkUserIdentity", + "id": "ms-" + tc.fromUser.Username, + "displayName": tc.fromUser.Username, + "userIdentityType": "aadUser", + "tenantId": "tenant-id", + }, + }, + "body": map[string]any{ + "contentType": "text", + "content": "Hello World", + }, + "channelIdentity": map[string]any{ + "channelId": newDMID, + }, } - newReaction, _, err := client.SaveReaction(context.Background(), &reaction) + + err = mockClient.MockBatch("edit-message", []containere2e.BatchRequest{ + { + Method: http.MethodPatch, + URL: "/chats/" + newDMID + "/messages/" + newPostID, + }, { + Method: http.MethodGet, + URL: "/chats/" + newDMID + "/messages/" + newPostID, + }}, + []containere2e.BatchResponse{ + { + StatusCode: http.StatusNoContent, + Body: fakePost, + }, + { + StatusCode: http.StatusOK, + Body: fakePost, + }, + }, + ) require.NoError(t, err) - t.Run("add reaction", func(t *testing.T) { - require.EventuallyWithT(t, func(c *assert.CollectT) { - if enabledSelectiveSync && tc.expectedWithSelectiveSync || !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { - assert.NoError(c, mockClient.Assert("set-reaction", 1)) - } else { - assert.NoError(c, mockClient.Assert("set-reaction", 0)) - } - assert.NoError(c, mockClient.Assert("unset-reaction", 0)) - }, 5*time.Second, 200*time.Millisecond) - }) + require.NoError(t, mockClient.Get("get-posted-message", "/v1.0/chats/"+newDMID+"/messages/"+newPostID, fakePost)) - _, err = client.DeleteReaction(context.Background(), newReaction) + err = mockClient.MockBatch("set-reaction", []containere2e.BatchRequest{ + { + Method: http.MethodPost, + URL: "/chats/" + newDMID + "/messages/" + newPostID + "/setReaction", + }, { + Method: http.MethodGet, + URL: "/chats/" + newDMID + "/messages/" + newPostID, + }}, + []containere2e.BatchResponse{ + { + StatusCode: http.StatusNoContent, + }, + { + StatusCode: http.StatusOK, + Body: fakePost, + }, + }) require.NoError(t, err) - t.Run("remove reaction", func(t *testing.T) { - require.EventuallyWithT(t, func(c *assert.CollectT) { - if enabledSelectiveSync && tc.expectedWithSelectiveSync || !enabledSelectiveSync && tc.expectedWithoutSelectiveSync { - assert.NoError(c, mockClient.Assert("set-reaction", 1)) - assert.NoError(c, mockClient.Assert("unset-reaction", 1)) - } else { - assert.NoError(c, mockClient.Assert("set-reaction", 0)) + err = mockClient.MockBatch("unset-reaction", []containere2e.BatchRequest{ + { + Method: http.MethodPost, + URL: "/chats/" + newDMID + "/messages/" + newPostID + "/unsetReaction", + }, { + Method: http.MethodGet, + URL: "/chats/" + newDMID + "/messages/" + newPostID, + }}, + []containere2e.BatchResponse{ + { + StatusCode: http.StatusNoContent, + }, + { + StatusCode: http.StatusOK, + Body: fakePost, + }, + }) + require.NoError(t, err) + + post := model.Post{ + UserId: tc.fromUser.Id, + ChannelId: dm.Id, + Message: "message", + } + + t.Run(tc.name, func(t *testing.T) { + newPost, _, err := client.CreatePost(context.Background(), &post) + require.NoError(t, err) + + t.Run("new post", func(t *testing.T) { + require.EventuallyWithT(t, func(c *assert.CollectT) { + if (enabledSelectiveSync && syncRemoteOnly && tc.expectedWithSelectiveSyncRemoteOnly) || + (enabledSelectiveSync && !syncRemoteOnly && tc.expectedWithSelectiveSync) || + (!enabledSelectiveSync && tc.expectedWithoutSelectiveSync) { + assert.NoError(c, mockClient.Assert("post-message", 1)) + } else { + assert.NoError(c, mockClient.Assert("post-message", 0)) + } + }, 5*time.Second, 200*time.Millisecond) + }) + + reply := model.Post{ + UserId: tc.fromUser.Id, + ChannelId: dm.Id, + Message: "reply", + RootId: newPost.Id, + } + newReply, _, err := client.CreatePost(context.Background(), &reply) + require.NoError(t, err) + + t.Run("reply", func(t *testing.T) { + require.EventuallyWithT(t, func(c *assert.CollectT) { + if (enabledSelectiveSync && syncRemoteOnly && tc.expectedWithSelectiveSyncRemoteOnly) || + (enabledSelectiveSync && !syncRemoteOnly && tc.expectedWithSelectiveSync) || + (!enabledSelectiveSync && tc.expectedWithoutSelectiveSync) { + assert.NoError(c, mockClient.Assert("post-message", 2)) + } else { + assert.NoError(c, mockClient.Assert("post-message", 0)) + } + }, 5*time.Second, 200*time.Millisecond) + }) + + newPost.Message = "edited message" + _, _, err = client.UpdatePost(context.Background(), newPost.Id, newPost) + require.NoError(t, err) + + t.Run("edit", func(t *testing.T) { + require.EventuallyWithT(t, func(c *assert.CollectT) { + if (enabledSelectiveSync && syncRemoteOnly && tc.expectedWithSelectiveSyncRemoteOnly) || + (enabledSelectiveSync && !syncRemoteOnly && tc.expectedWithSelectiveSync) || + (!enabledSelectiveSync && tc.expectedWithoutSelectiveSync) { + assert.NoError(c, mockClient.Assert("edit-message", 1)) + } else { + assert.NoError(c, mockClient.Assert("edit-message", 0)) + } + }, 5*time.Second, 200*time.Millisecond) + }) + + newReply.Message = "edited reply" + _, _, err = client.UpdatePost(context.Background(), newReply.Id, newReply) + require.NoError(t, err) + + t.Run("edit reply", func(t *testing.T) { + require.EventuallyWithT(t, func(c *assert.CollectT) { + if (enabledSelectiveSync && syncRemoteOnly && tc.expectedWithSelectiveSyncRemoteOnly) || + (enabledSelectiveSync && !syncRemoteOnly && tc.expectedWithSelectiveSync) || + (!enabledSelectiveSync && tc.expectedWithoutSelectiveSync) { + assert.NoError(c, mockClient.Assert("edit-message", 2)) + } else { + assert.NoError(c, mockClient.Assert("edit-message", 0)) + } + }, 5*time.Second, 200*time.Millisecond) + }) + + reaction := model.Reaction{ + UserId: tc.fromUser.Id, + EmojiName: "heart", + PostId: newPost.Id, + ChannelId: dm.Id, + } + newReaction, _, err := client.SaveReaction(context.Background(), &reaction) + require.NoError(t, err) + + t.Run("add reaction", func(t *testing.T) { + require.EventuallyWithT(t, func(c *assert.CollectT) { + if (enabledSelectiveSync && syncRemoteOnly && tc.expectedWithSelectiveSyncRemoteOnly) || + (enabledSelectiveSync && !syncRemoteOnly && tc.expectedWithSelectiveSync) || + (!enabledSelectiveSync && tc.expectedWithoutSelectiveSync) { + assert.NoError(c, mockClient.Assert("set-reaction", 1)) + } else { + assert.NoError(c, mockClient.Assert("set-reaction", 0)) + } assert.NoError(c, mockClient.Assert("unset-reaction", 0)) - } - }, 5*time.Second, 200*time.Millisecond) + }, 5*time.Second, 200*time.Millisecond) + }) + + _, err = client.DeleteReaction(context.Background(), newReaction) + require.NoError(t, err) + + t.Run("remove reaction", func(t *testing.T) { + require.EventuallyWithT(t, func(c *assert.CollectT) { + if (enabledSelectiveSync && syncRemoteOnly && tc.expectedWithSelectiveSyncRemoteOnly) || + (enabledSelectiveSync && !syncRemoteOnly && tc.expectedWithSelectiveSync) || + (!enabledSelectiveSync && tc.expectedWithoutSelectiveSync) { + assert.NoError(c, mockClient.Assert("set-reaction", 1)) + assert.NoError(c, mockClient.Assert("unset-reaction", 1)) + } else { + assert.NoError(c, mockClient.Assert("set-reaction", 0)) + assert.NoError(c, mockClient.Assert("unset-reaction", 0)) + } + }, 5*time.Second, 200*time.Millisecond) + }) }) - }) - } - }) + } + }) + } } } diff --git a/server/command.go b/server/command.go index 097b663bb..6c624c434 100644 --- a/server/command.go +++ b/server/command.go @@ -530,7 +530,7 @@ func (p *Plugin) executeDisconnectCommand(args *model.CommandArgs) (*model.Comma return p.cmdSuccess(args, "Error: the account is not connected") } - if _, err = p.store.GetTokenForMattermostUser(args.UserId); err != nil { + if token, _ := p.store.GetTokenForMattermostUser(args.UserId); token == nil { return p.cmdSuccess(args, "Error: the account is not connected") } @@ -626,7 +626,7 @@ func (p *Plugin) executeNotificationsCommand(args *model.CommandArgs, parameters return p.cmdSuccess(args, "Invalid notifications command, one argument is required.") } - isConnected, err := p.isUserConnected(args.UserId) + isConnected, err := p.IsUserConnected(args.UserId) if err != nil { p.API.LogWarn("unable to check if the user is connected", "error", err.Error()) return p.cmdError(args, "Error: Unable to get the connection status") diff --git a/server/configuration.go b/server/configuration.go index 22de07dc1..94184e759 100644 --- a/server/configuration.go +++ b/server/configuration.go @@ -36,6 +36,7 @@ type configuration struct { SyncFileAttachments bool `json:"syncfileattachments"` SyncUsers int `json:"syncusers"` SyncGuestUsers bool `json:"syncGuestUsers"` + SyncRemoteOnly bool `json:"syncRemoteOnly"` CertificatePublic string `json:"certificatepublic"` CertificateKey string `json:"certificatekey"` MaxSizeForCompleteDownload int `json:"maxSizeForCompleteDownload"` diff --git a/server/handlers.go b/server/handlers.go index 3c8f68957..362a0f6bf 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -235,7 +235,6 @@ func (ah *ActivityHandler) handleCreatedActivity(msg *clientmodels.Message, subs ah.plugin.GetAPI().LogWarn("Unable to get original message", "error", err.Error()) return metrics.DiscardedReasonUnableToGetTeamsData } - if msg == nil { return metrics.DiscardedReasonUnableToGetTeamsData } @@ -260,8 +259,8 @@ func (ah *ActivityHandler) handleCreatedActivity(msg *clientmodels.Message, subs return metrics.DiscardedReasonDuplicatedPost } - msteamsUserID, _ := ah.plugin.GetStore().MattermostToTeamsUserID(ah.plugin.GetBotUserID()) - if msg.UserID == msteamsUserID { + msteamsBotUserID, _ := ah.plugin.GetStore().MattermostToTeamsUserID(ah.plugin.GetBotUserID()) + if msg.UserID == msteamsBotUserID { return metrics.DiscardedReasonIsBotUser } @@ -297,16 +296,15 @@ func (ah *ActivityHandler) handleCreatedActivity(msg *clientmodels.Message, subs return metrics.DiscardedReasonOther } + senderID, _ = ah.plugin.GetStore().TeamsToMattermostUserID(msg.UserID) if ah.plugin.GetSelectiveSync() { - if shouldSync, appErr := ah.plugin.ChatShouldSync(channelID); appErr != nil { - ah.plugin.GetAPI().LogWarn("Failed to determine if shouldSyncChat", "channel_id", channelID, "error", appErr.Error()) + if shouldSync, errSync := ah.plugin.ChannelShouldSyncCreated(channelID, senderID); errSync != nil { + ah.plugin.GetAPI().LogWarn("Unable to determine if channel should sync", "error", errSync.Error()) return metrics.DiscardedReasonOther } else if !shouldSync { return metrics.DiscardedReasonSelectiveSync } } - - senderID, _ = ah.plugin.GetStore().TeamsToMattermostUserID(msg.UserID) } else { if !ah.plugin.GetSyncLinkedChannels() { // Skipping because linked channels are disabled @@ -416,8 +414,8 @@ func (ah *ActivityHandler) handleUpdatedActivity(msg *clientmodels.Message, subs return metrics.DiscardedReasonNotUserEvent } - msteamsUserID, _ := ah.plugin.GetStore().MattermostToTeamsUserID(ah.plugin.GetBotUserID()) - if msg.UserID == msteamsUserID { + msteamsBotUserID, _ := ah.plugin.GetStore().MattermostToTeamsUserID(ah.plugin.GetBotUserID()) + if msg.UserID == msteamsBotUserID { return metrics.DiscardedReasonIsBotUser } diff --git a/server/handlers_test.go b/server/handlers_test.go index 92dade0bf..e0b788491 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -15,8 +15,8 @@ import ( "github.com/mattermost/mattermost-plugin-msteams/server/testutils" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/plugin/plugintest" + "github.com/mattermost/mattermost/server/public/plugin/plugintest/mock" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) diff --git a/server/message_hooks.go b/server/message_hooks.go index 877aee44c..4f8a0d831 100644 --- a/server/message_hooks.go +++ b/server/message_hooks.go @@ -65,8 +65,8 @@ func (p *Plugin) MessageHasBeenDeleted(_ *plugin.Context, post *model.Post) { } if p.getConfiguration().SelectiveSync { - shouldSync, appErr := p.ChatShouldSync(post.ChannelId) - if appErr != nil { + shouldSync, err := p.ChannelShouldSync(post.ChannelId) + if err != nil { p.API.LogWarn("Failed to check if chat should be synced", "error", appErr.Error(), "post_id", post.Id, "channel_id", post.ChannelId) return } else if !shouldSync { @@ -129,26 +129,21 @@ func (p *Plugin) MessageHasBeenPosted(_ *plugin.Context, post *model.Post) { return } - isSelfPost := len(members) == 1 - chatMembersSpanPlatforms := false - if !isSelfPost { - chatMembersSpanPlatforms, err = p.ChatMembersSpanPlatforms(members) + chatShouldSync := false + if p.getConfiguration().SelectiveSync { + chatShouldSync, err = p.ChannelShouldSync(post.ChannelId) if err != nil { - p.API.LogWarn("Failed to check if chat members span platforms", "error", err.Error(), "post_id", post.Id, "channel_id", post.ChannelId) return - } - - if p.getConfiguration().SelectiveSync && !chatMembersSpanPlatforms { + } else if !chatShouldSync { return } } - dstUsers := []string{} for _, m := range members { dstUsers = append(dstUsers, m.UserId) } - _, err = p.SendChat(post.UserId, dstUsers, post, chatMembersSpanPlatforms) + _, err = p.SendChat(post.UserId, dstUsers, post, chatShouldSync) if err != nil { p.API.LogWarn("Unable to handle message sent", "error", err.Error()) } @@ -532,7 +527,7 @@ func (p *Plugin) UnsetReaction(teamID, channelID, userID string, post *model.Pos return nil } -func (p *Plugin) SendChat(srcUser string, usersIDs []string, post *model.Post, chatMembersSpanPlatforms bool) (string, error) { +func (p *Plugin) SendChat(srcUser string, usersIDs []string, post *model.Post, containsRemoteUser bool) (string, error) { parentID := "" if post.RootId != "" { parentInfo, _ := p.store.GetPostInfoByMattermostID(post.RootId) @@ -543,7 +538,7 @@ func (p *Plugin) SendChat(srcUser string, usersIDs []string, post *model.Post, c _, err := p.store.MattermostToTeamsUserID(srcUser) if err != nil { - if chatMembersSpanPlatforms { + if containsRemoteUser { p.handlePromptForConnection(srcUser, post.ChannelId) } return "", err @@ -551,7 +546,7 @@ func (p *Plugin) SendChat(srcUser string, usersIDs []string, post *model.Post, c client, err := p.GetClientForUser(srcUser) if err != nil { - if chatMembersSpanPlatforms { + if containsRemoteUser { p.handlePromptForConnection(srcUser, post.ChannelId) } return "", err diff --git a/server/metrics/metrics.go b/server/metrics/metrics.go index 0cc6d52d9..fe1838aae 100644 --- a/server/metrics/metrics.go +++ b/server/metrics/metrics.go @@ -56,6 +56,7 @@ const ( DiscardedReasonFailedSubscriptionCheck = "failed_subscription_check" DiscardedReasonFailedToRefresh = "failed_to_refresh" DiscardedReasonSelectiveSync = "selective_sync" + DiscardedReasonUserNotConnected = "user_not_connected" DiscardedReasonGeneratedFromMattermost = "generated_from_mattermost" DiscardedReasonNotificationsOnly = "notifications_only" DiscardedReasonChannelNotificationsUnsupported = "channel_notifications_unsupported" diff --git a/server/plugin.go b/server/plugin.go index 439a79b17..fffeac77d 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "database/sql" "encoding/base64" "encoding/pem" "fmt" @@ -159,6 +160,10 @@ func (p *Plugin) GetSelectiveSync() bool { return p.getConfiguration().SelectiveSync } +func (p *Plugin) GetSyncRemoteOnly() bool { + return p.getConfiguration().SyncRemoteOnly +} + func (p *Plugin) MessageFingerprint() string { return "" } @@ -898,6 +903,14 @@ func (p *Plugin) IsRemoteUser(user *model.User) bool { return user.RemoteId != nil && *user.RemoteId == p.remoteID } +func (p *Plugin) IsUserConnected(userID string) (bool, error) { + token, err := p.store.GetTokenForMattermostUser(userID) + if err != nil && err != sql.ErrNoRows { + return false, errors.Wrap(err, "Unable to determine if user is connected to MS Teams") + } + return token != nil, nil +} + func (p *Plugin) GetRemoteID() string { return p.remoteID } diff --git a/server/selective_sync.go b/server/selective_sync.go index fa6b3448b..23bcf625e 100644 --- a/server/selective_sync.go +++ b/server/selective_sync.go @@ -3,30 +3,46 @@ package main import ( "math" + "github.com/mattermost/mattermost-plugin-msteams/server/store/storemodels" "github.com/mattermost/mattermost/server/public/model" + "github.com/pkg/errors" ) -// ChatShouldSync determines if the members of the given channel span both Mattermost and -// MS Teams. Chats between users on the same platform are skipped if selective sync is enabled. -// Chats with only a single member are self chats and always sync. -func (p *Plugin) ChatShouldSync(channelID string) (bool, error) { +func (p *Plugin) ChannelShouldSyncCreated(channelID, senderID string) (bool, error) { + if senderID == "" { + return false, errors.New("Invalid function call, requires RemoteOnly and a senderID") + } + + if p.GetSyncRemoteOnly() { + return p.ChannelConnectedOrRemote(channelID, senderID) + } + return p.ChannelShouldSync(channelID) +} + +func (p *Plugin) ChannelShouldSync(channelID string) (bool, error) { members, err := p.apiClient.Channel.ListMembers(channelID, 0, math.MaxInt32) if err != nil { return false, err } + if len(members) == 1 { return true, nil } + if p.GetSyncRemoteOnly() { + return p.MembersContainsRemote(members) + } return p.ChatMembersSpanPlatforms(members) } // ChatMembersSpanPlatforms determines if the given channel members span both Mattermost and -// MS Teams. Only Synthetic users are considered MS Teams users.// Chats between users on the same platform are skipped if selective sync is enabled. +// MS Teams. Chats between users on the same platform are skipped if selective sync is enabled. func (p *Plugin) ChatMembersSpanPlatforms(members []*model.ChannelMember) (bool, error) { - if len(members) <= 1 { - return false, &model.AppError{Message: "Invalid function call, requires multiple members"} + if len(members) == 1 { + return false, errors.New("Invalid function call, requires multiple members") } + atLeastOneLocalUser := false + atLeastOneRemoteUser := false for _, m := range members { user, err := p.apiClient.User.Get(m.UserId) if err != nil { @@ -35,9 +51,75 @@ func (p *Plugin) ChatMembersSpanPlatforms(members []*model.ChannelMember) (bool, if p.IsRemoteUser(user) { // Synthetic users are always remote. + atLeastOneRemoteUser = true + } else if p.getPrimaryPlatform(user.Id) == storemodels.PreferenceValuePlatformMSTeams { + // Treat Teams primary users as remote + atLeastOneRemoteUser = true + } else { + // Otherwise the user is considered local. + atLeastOneLocalUser = true + } + + if atLeastOneLocalUser && atLeastOneRemoteUser { + return true, nil + } + } + return false, nil +} + +func (p *Plugin) ChannelConnectedOrRemote(channelID, senderID string) (bool, error) { + senderConnected, err := p.IsUserConnected(senderID) + if err != nil { + return false, err + } + members, err := p.apiClient.Channel.ListMembers(channelID, 0, math.MaxInt32) + if err != nil { + return false, err + } + if len(members) == 1 { + return true, nil + } + + if senderConnected { + containsRemote, memberErr := p.MembersContainsRemote(members) + if memberErr != nil { + return false, memberErr + } + return containsRemote, nil + } + + senderMember := &model.ChannelMember{ + UserId: senderID, + } + senderRemote, err := p.MembersContainsRemote([]*model.ChannelMember{senderMember}) + if err != nil { + return false, err + } + if !senderRemote { + return false, nil + } + for _, m := range members { + isConnected, err := p.IsUserConnected(m.UserId) + if err != nil { + return false, err + } else if isConnected { return true, nil } } + return false, nil +} +// MembersContainsRemote determines if any of the given channel members are remote. +func (p *Plugin) MembersContainsRemote(members []*model.ChannelMember) (bool, error) { + for _, m := range members { + user, err := p.apiClient.User.Get(m.UserId) + if err != nil { + return false, err + } + + if p.IsRemoteUser(user) { + return true, nil + } + } return false, nil } diff --git a/server/selective_sync_test.go b/server/selective_sync_test.go index 41ce11a26..f932e0b1e 100644 --- a/server/selective_sync_test.go +++ b/server/selective_sync_test.go @@ -9,29 +9,33 @@ import ( "github.com/stretchr/testify/require" ) -func TestChatShouldSync(t *testing.T) { +func TestChatMembersSpanPlatforms(t *testing.T) { th := setupTestHelper(t) - t.Run("invalid channel id", func(t *testing.T) { - _, err := th.p.ChatShouldSync("") - require.Error(t, err) - }) - - t.Run("unknown channel id", func(t *testing.T) { - _, err := th.p.ChatShouldSync(model.NewId()) - require.Error(t, err) + t.Run("empty set of channel members", func(t *testing.T) { + chatMembersSpanPlatforms, appErr := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{}) + require.Nil(t, appErr) + require.False(t, chatMembersSpanPlatforms) }) - t.Run("dm with a single users", func(t *testing.T) { + t.Run("single user", func(t *testing.T) { team := th.SetupTeam(t) user1 := th.SetupRemoteUser(t, team) - channel, appErr := th.p.API.GetDirectChannel(user1.Id, user1.Id) - require.Nil(t, appErr) + chatShouldSync, appErr := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + }) - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) - require.NoError(t, err) - assert.True(t, chatShouldSync) + require.Error(t, appErr) + assert.False(t, chatShouldSync) + }) + + t.Run("user with empty id", func(t *testing.T) { + team := th.SetupTeam(t) + user1 := th.SetupRemoteUser(t, team) + chatMembersSpanPlatforms, appErr := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{{UserId: user1.Id}, {UserId: ""}}) + require.Error(t, appErr) + require.False(t, chatMembersSpanPlatforms) }) t.Run("dm between two local users", func(t *testing.T) { @@ -39,11 +43,11 @@ func TestChatShouldSync(t *testing.T) { user1 := th.SetupUser(t, team) user2 := th.SetupUser(t, team) - channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) + chatShouldSync, appErr := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + }) require.Nil(t, appErr) - - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) - require.NoError(t, err) assert.False(t, chatShouldSync) }) @@ -62,12 +66,12 @@ func TestChatShouldSync(t *testing.T) { user2, appErr = th.p.API.UpdateUser(user2) require.Nil(t, appErr) - channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) - require.Nil(t, appErr) - - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) + chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + }) require.NoError(t, err) - assert.True(t, chatShouldSync) + assert.False(t, chatShouldSync) }) t.Run("dm between a local and a remote user", func(t *testing.T) { @@ -81,10 +85,10 @@ func TestChatShouldSync(t *testing.T) { user1, appErr = th.p.API.UpdateUser(user1) require.Nil(t, appErr) - channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) - require.Nil(t, appErr) - - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) + chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + }) require.NoError(t, err) assert.True(t, chatShouldSync) }) @@ -97,12 +101,15 @@ func TestChatShouldSync(t *testing.T) { err := th.p.setPrimaryPlatform(user2.Id, storemodels.PreferenceValuePlatformMSTeams) require.NoError(t, err) - channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) + _, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) require.Nil(t, appErr) - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) - require.NoError(t, err) - assert.False(t, chatShouldSync) + chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + }) + require.Nil(t, err) + assert.True(t, chatShouldSync) }) t.Run("gm between three local users", func(t *testing.T) { @@ -111,11 +118,12 @@ func TestChatShouldSync(t *testing.T) { user2 := th.SetupUser(t, team) user3 := th.SetupUser(t, team) - channel, appErr := th.p.API.GetGroupChannel([]string{user1.Id, user2.Id, user3.Id}) - require.Nil(t, appErr) - - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) - require.NoError(t, err) + chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + {UserId: user3.Id}, + }) + require.Nil(t, err) assert.False(t, chatShouldSync) }) @@ -139,12 +147,13 @@ func TestChatShouldSync(t *testing.T) { user3, appErr = th.p.API.UpdateUser(user3) require.Nil(t, appErr) - channel, appErr := th.p.API.GetGroupChannel([]string{user1.Id, user2.Id, user3.Id}) - require.Nil(t, appErr) - - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) + chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + {UserId: user3.Id}, + }) require.NoError(t, err) - assert.True(t, chatShouldSync) + assert.False(t, chatShouldSync) }) t.Run("gm between a mixture of local and remote users", func(t *testing.T) { @@ -163,10 +172,11 @@ func TestChatShouldSync(t *testing.T) { user3, appErr = th.p.API.UpdateUser(user3) require.Nil(t, appErr) - channel, appErr := th.p.API.GetGroupChannel([]string{user1.Id, user2.Id, user3.Id}) - require.Nil(t, appErr) - - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) + chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + {UserId: user3.Id}, + }) require.NoError(t, err) assert.True(t, chatShouldSync) }) @@ -180,21 +190,25 @@ func TestChatShouldSync(t *testing.T) { err := th.p.setPrimaryPlatform(user3.Id, storemodels.PreferenceValuePlatformMSTeams) require.NoError(t, err) - channel, appErr := th.p.API.GetGroupChannel([]string{user1.Id, user2.Id, user3.Id}) + _, appErr := th.p.API.GetGroupChannel([]string{user1.Id, user2.Id, user3.Id}) require.Nil(t, appErr) - chatShouldSync, err := th.p.ChatShouldSync(channel.Id) + chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + {UserId: user1.Id}, + {UserId: user2.Id}, + {UserId: user3.Id}, + }) require.NoError(t, err) - assert.False(t, chatShouldSync) + assert.True(t, chatShouldSync) }) } -func TestChatMembersSpanPlatforms(t *testing.T) { +func TestMembersContainRemote(t *testing.T) { th := setupTestHelper(t) t.Run("empty set of channel members", func(t *testing.T) { - chatMembersSpanPlatforms, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{}) - require.Error(t, err) + chatMembersSpanPlatforms, err := th.p.MembersContainsRemote([]*model.ChannelMember{}) + require.NoError(t, err) require.False(t, chatMembersSpanPlatforms) }) @@ -202,18 +216,18 @@ func TestChatMembersSpanPlatforms(t *testing.T) { team := th.SetupTeam(t) user1 := th.SetupRemoteUser(t, team) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, }) - require.Error(t, err) - assert.False(t, chatShouldSync) + require.NoError(t, err) + assert.True(t, remoteUsers) }) t.Run("user with empty id", func(t *testing.T) { team := th.SetupTeam(t) user1 := th.SetupRemoteUser(t, team) - chatMembersSpanPlatforms, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + chatMembersSpanPlatforms, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: ""}, {UserId: user1.Id}}) require.Error(t, err) @@ -225,12 +239,12 @@ func TestChatMembersSpanPlatforms(t *testing.T) { user1 := th.SetupUser(t, team) user2 := th.SetupUser(t, team) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, }) require.NoError(t, err) - assert.False(t, chatShouldSync) + assert.False(t, remoteUsers) }) t.Run("dm between two remote users", func(t *testing.T) { @@ -248,12 +262,12 @@ func TestChatMembersSpanPlatforms(t *testing.T) { user2, appErr = th.p.API.UpdateUser(user2) require.Nil(t, appErr) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, }) require.NoError(t, err) - assert.True(t, chatShouldSync) + assert.True(t, remoteUsers) }) t.Run("dm between a local and a remote user", func(t *testing.T) { @@ -261,12 +275,12 @@ func TestChatMembersSpanPlatforms(t *testing.T) { user1 := th.SetupRemoteUser(t, team) user2 := th.SetupUser(t, team) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, }) require.NoError(t, err) - assert.True(t, chatShouldSync) + assert.True(t, remoteUsers) }) t.Run("dm between a local and a local user with teams as primary platform", func(t *testing.T) { @@ -280,12 +294,12 @@ func TestChatMembersSpanPlatforms(t *testing.T) { _, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) require.Nil(t, appErr) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, }) require.NoError(t, err) - assert.False(t, chatShouldSync) + assert.False(t, remoteUsers) }) t.Run("gm between three local users", func(t *testing.T) { @@ -294,13 +308,13 @@ func TestChatMembersSpanPlatforms(t *testing.T) { user2 := th.SetupUser(t, team) user3 := th.SetupUser(t, team) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, {UserId: user3.Id}, }) require.NoError(t, err) - assert.False(t, chatShouldSync) + assert.False(t, remoteUsers) }) t.Run("gm between three remote users", func(t *testing.T) { @@ -323,13 +337,13 @@ func TestChatMembersSpanPlatforms(t *testing.T) { user3, appErr = th.p.API.UpdateUser(user3) require.Nil(t, appErr) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, {UserId: user3.Id}, }) require.NoError(t, err) - assert.True(t, chatShouldSync) + assert.True(t, remoteUsers) }) t.Run("gm between a mixture of local and remote users", func(t *testing.T) { @@ -348,13 +362,13 @@ func TestChatMembersSpanPlatforms(t *testing.T) { user3, appErr = th.p.API.UpdateUser(user3) require.Nil(t, appErr) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, {UserId: user3.Id}, }) require.NoError(t, err) - assert.True(t, chatShouldSync) + assert.True(t, remoteUsers) }) t.Run("gm between two local users and a local user with teams as primary platform", func(t *testing.T) { @@ -369,12 +383,116 @@ func TestChatMembersSpanPlatforms(t *testing.T) { _, appErr := th.p.API.GetGroupChannel([]string{user1.Id, user2.Id, user3.Id}) require.Nil(t, appErr) - chatShouldSync, err := th.p.ChatMembersSpanPlatforms([]*model.ChannelMember{ + remoteUsers, err := th.p.MembersContainsRemote([]*model.ChannelMember{ {UserId: user1.Id}, {UserId: user2.Id}, {UserId: user3.Id}, }) require.NoError(t, err) - assert.False(t, chatShouldSync) + assert.False(t, remoteUsers) + }) +} + +func TestChannelConnectedOrRemote(t *testing.T) { + th := setupTestHelper(t) + + t.Run("unknown sender id", func(t *testing.T) { + remoteUsers, err := th.p.ChannelConnectedOrRemote("", model.NewId()) + require.Error(t, err) + assert.False(t, remoteUsers) + }) + + t.Run("unknown channel id", func(t *testing.T) { + remoteUsers, err := th.p.ChannelConnectedOrRemote(model.NewId(), "") + require.Error(t, err) + assert.False(t, remoteUsers) + }) + + t.Run("dm with a single users", func(t *testing.T) { + team := th.SetupTeam(t) + user1 := th.SetupUser(t, team) + + channel, appErr := th.p.API.GetDirectChannel(user1.Id, user1.Id) + require.Nil(t, appErr) + + remoteUsers, err := th.p.ChannelConnectedOrRemote(channel.Id, user1.Id) + require.NoError(t, err) + assert.True(t, remoteUsers) + }) + + t.Run("dm between two local users", func(t *testing.T) { + team := th.SetupTeam(t) + user1 := th.SetupUser(t, team) + th.ConnectUser(t, user1.Id) + user2 := th.SetupUser(t, team) + th.ConnectUser(t, user2.Id) + + channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) + require.Nil(t, appErr) + + remoteUsers, err := th.p.ChannelConnectedOrRemote(channel.Id, user1.Id) + require.NoError(t, err) + assert.False(t, remoteUsers) + }) + + t.Run("dm between two remote users", func(t *testing.T) { + team := th.SetupTeam(t) + user1 := th.SetupRemoteUser(t, team) + user2 := th.SetupRemoteUser(t, team) + + var appErr *model.AppError + + user1.RemoteId = model.NewString(th.p.remoteID) + user1, appErr = th.p.API.UpdateUser(user1) + require.Nil(t, appErr) + + user2.RemoteId = model.NewString(th.p.remoteID) + user2, appErr = th.p.API.UpdateUser(user2) + require.Nil(t, appErr) + + channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) + require.Nil(t, appErr) + + remoteUsers, err := th.p.ChannelConnectedOrRemote(channel.Id, user1.Id) + require.NoError(t, err) + assert.False(t, remoteUsers) + }) + + t.Run("dm between a local and a remote user", func(t *testing.T) { + team := th.SetupTeam(t) + user1 := th.SetupUser(t, team) + th.ConnectUser(t, user1.Id) + + user2 := th.SetupRemoteUser(t, team) + var appErr *model.AppError + user2.RemoteId = model.NewString(th.p.remoteID) + user2, appErr = th.p.API.UpdateUser(user1) + require.Nil(t, appErr) + + channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) + require.Nil(t, appErr) + + remoteUsers, err := th.p.ChannelConnectedOrRemote(channel.Id, user1.Id) + require.NoError(t, err) + assert.True(t, remoteUsers) + }) + + t.Run("dm between a remote and a local user", func(t *testing.T) { + team := th.SetupTeam(t) + user1 := th.SetupRemoteUser(t, team) + var appErr *model.AppError + user1.RemoteId = model.NewString(th.p.remoteID) + user1, appErr = th.p.API.UpdateUser(user1) + require.Nil(t, appErr) + + user2 := th.SetupUser(t, team) + th.ConnectUser(t, user2.Id) + + channel, appErr := th.p.API.GetDirectChannel(user1.Id, user2.Id) + require.Nil(t, appErr) + + remoteUsers, err := th.p.ChannelConnectedOrRemote(channel.Id, user1.Id) + require.NoError(t, err) + assert.True(t, remoteUsers) }) } diff --git a/server/store/sqlstore/store.go b/server/store/sqlstore/store.go index 1cf5c0d1b..d65fd2c2f 100644 --- a/server/store/sqlstore/store.go +++ b/server/store/sqlstore/store.go @@ -362,7 +362,7 @@ func (s *SQLStore) getTokenForMattermostUser(db sq.BaseRunner, userID string) (* } if encryptedToken == "" { - return nil, errors.New("token not found") + return nil, nil } tokendata, err := decrypt(s.encryptionKey(), encryptedToken) @@ -371,7 +371,7 @@ func (s *SQLStore) getTokenForMattermostUser(db sq.BaseRunner, userID string) (* } if tokendata == "" { - return nil, errors.New("token not found") + return nil, nil } var token oauth2.Token