MM-58437: support notifications alongside channel sync #674
Conversation
lieut-data
added
the
Do Not Merge/Awaiting PR
| @@ -558,9 +558,9 @@ func getUpdatedMessage(teamID, channelID, parentID, msteamsID string, client mst | |||
| func (p *Plugin) ShouldSyncDMGMChannel(channel *model.Channel) bool { | |||
| switch channel.Type { | |||
| case model.ChannelTypeDirect: | |||
| return p.getConfiguration().SyncDirectMessages | |||
| return p.GetSyncDirectMessages() | |||
There was a problem hiding this comment.
We use the helper functions, since they now interpolate whether or not notifications are enabled to decide if we should sync direct messages.
| // Ignore two-way sync when notifications are enabled. | ||
| if p.GetSyncNotifications() { | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
ShouldSyncDMGMChannel handles this for us now.
| "github.com/mattermost/mattermost/server/public/model" | ||
| ) | ||
|
|
||
| // mockTeams is an abstraction over directly mocking the client calls made by the plugin to instead |
There was a problem hiding this comment.
Another new experiment: most of the time, I don't actually /care/ about the function calls, just want to get the side effects. This streamlines that step. If it works, I'll extend this and make it part of the other tests that aren't explicitly testing function calls in the same way.
| func (mth *mockTeamsHelper) registerChat(chatID string, users []*model.User) { | ||
| var members []clientmodels.ChatMember | ||
| for _, user := range users { | ||
| mth.registerUser(user) |
There was a problem hiding this comment.
Automatically teach the mock about the user!
| }, nil).Maybe() | ||
| } | ||
|
|
||
| func (mth *mockTeamsHelper) registerGroupChat(chatID string, users []*model.User) { |
There was a problem hiding this comment.
I debated making this "smart" based on the number of users, but figured I'd keep it 1:1 with the return values for now.
| @@ -122,11 +122,11 @@ func (p *Plugin) GetSyncNotifications() bool { | |||
| } | |||
|
|
|||
| func (p *Plugin) GetSyncDirectMessages() bool { | |||
There was a problem hiding this comment.
I know it's not part of this PR pre se but I'd like to have those renamed to something more accurate.
…fications-alongside-channel-sync
lieut-data
removed
the
Do Not Merge/Awaiting PR
lieut-data
deleted the
mm-58437-support-notifications-alongside-channel-sync
branch
Summary
Allow normal channel sync alongside notifications. Sync of chats and group chats remains disabled when notifications are enabled.
Ticket Link
Fixes: https://mattermost.atlassian.net/browse/MM-58437