From db01f2a91b67e24187294dbe30cca1cf8fc6e494 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 2 Apr 2021 14:33:23 +0530 Subject: [PATCH] MM-34002: Improve AddUserToChannel (#17174) * MM-34002: Improve AddUserToChannel When we would add a user to a channel, we would check whether the user is removed from that team or not. During LDAP sync, this check is not required because the team member would have just been created. Hence, we pass a boolean flag to bypass the check. And with that done, we can freely query the replica. https://mattermost.atlassian.net/browse/MM-34002 ```release-note NONE ``` * Refactor code * Rename a struct field * fix double negative --- api4/apitestlib.go | 18 +++---- api4/channel.go | 5 +- api4/channel_local.go | 2 +- api4/channel_test.go | 37 ++++++------- api4/post_test.go | 8 +-- api4/user_test.go | 12 ++--- app/app_iface.go | 6 ++- app/channel.go | 57 +++++++++++--------- app/channel_test.go | 57 ++++++++++---------- app/helper_test.go | 2 +- app/notification_test.go | 14 ++--- app/opentracing/opentracing_layer.go | 8 +-- app/plugin_api.go | 18 +++---- app/post_test.go | 4 +- app/slashcommands/command_invite.go | 4 +- app/slashcommands/command_invite_test.go | 3 +- app/slashcommands/command_leave_test.go | 8 +-- app/slashcommands/command_remove_test.go | 10 ++-- app/slashcommands/helper_test.go | 2 +- app/syncables.go | 4 +- app/syncables_test.go | 6 +-- app/team.go | 2 +- app/user.go | 2 +- app/user_test.go | 6 +-- cmd/mattermost/commands/channel.go | 2 +- migrations/helper_test.go | 2 +- services/sharedchannel/mock_AppIface_test.go | 14 ++--- services/sharedchannel/service.go | 2 +- services/sharedchannel/sync_recv.go | 2 +- services/slackimport/slackimport.go | 4 +- 30 files changed, 169 insertions(+), 152 deletions(-) diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 39abf6b7fe..cc457dd231 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -380,14 +380,14 @@ func (th *TestHelper) InitBasic() *TestHelper { th.BasicPost = th.CreatePost() th.LinkUserToTeam(th.BasicUser, th.BasicTeam) th.LinkUserToTeam(th.BasicUser2, th.BasicTeam) - th.App.AddUserToChannel(th.BasicUser, th.BasicChannel) - th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel) - th.App.AddUserToChannel(th.BasicUser, th.BasicChannel2) - th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel2) - th.App.AddUserToChannel(th.BasicUser, th.BasicPrivateChannel) - th.App.AddUserToChannel(th.BasicUser2, th.BasicPrivateChannel) - th.App.AddUserToChannel(th.BasicUser, th.BasicDeletedChannel) - th.App.AddUserToChannel(th.BasicUser2, th.BasicDeletedChannel) + th.App.AddUserToChannel(th.BasicUser, th.BasicChannel, false) + th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel, false) + th.App.AddUserToChannel(th.BasicUser, th.BasicChannel2, false) + th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel2, false) + th.App.AddUserToChannel(th.BasicUser, th.BasicPrivateChannel, false) + th.App.AddUserToChannel(th.BasicUser2, th.BasicPrivateChannel, false) + th.App.AddUserToChannel(th.BasicUser, th.BasicDeletedChannel, false) + th.App.AddUserToChannel(th.BasicUser2, th.BasicDeletedChannel, false) th.App.UpdateUserRoles(th.BasicUser.Id, model.SYSTEM_USER_ROLE_ID, false) th.Client.DeleteChannel(th.BasicDeletedChannel.Id) th.LoginBasic() @@ -792,7 +792,7 @@ func (th *TestHelper) LinkUserToTeam(user *model.User, team *model.Team) { func (th *TestHelper) AddUserToChannel(user *model.User, channel *model.Channel) *model.ChannelMember { utils.DisableDebugLogForTest() - member, err := th.App.AddUserToChannel(user, channel) + member, err := th.App.AddUserToChannel(user, channel, false) if err != nil { panic(err) } diff --git a/api4/channel.go b/api4/channel.go index 8e29c71880..aadc01c216 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -1540,7 +1540,10 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { } } - cm, err := c.App.AddChannelMember(member.UserId, channel, c.App.Session().UserId, postRootId) + cm, err := c.App.AddChannelMember(member.UserId, channel, app.ChannelMemberOpts{ + UserRequestorID: c.App.Session().UserId, + PostRootID: postRootId, + }) if err != nil { c.Err = err return diff --git a/api4/channel_local.go b/api4/channel_local.go index f0c3c54f18..6648d324f9 100644 --- a/api4/channel_local.go +++ b/api4/channel_local.go @@ -107,7 +107,7 @@ func localAddChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { } } - cm, err := c.App.AddUserToChannel(user, channel) + cm, err := c.App.AddUserToChannel(user, channel, false) if err != nil { c.Err = err return diff --git a/api4/channel_test.go b/api4/channel_test.go index 224907d8c1..2eea5419df 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/v5/app" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin/plugintest/mock" "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" @@ -1497,8 +1498,8 @@ func TestDeleteChannel(t *testing.T) { // successful delete of channel with multiple members publicChannel3 := th.CreatePublicChannel() - th.App.AddUserToChannel(user, publicChannel3) - th.App.AddUserToChannel(user2, publicChannel3) + th.App.AddUserToChannel(user, publicChannel3, false) + th.App.AddUserToChannel(user2, publicChannel3, false) _, resp = client.DeleteChannel(publicChannel3.Id) CheckNoError(t, resp) @@ -1574,9 +1575,9 @@ func TestDeleteChannel2(t *testing.T) { // channels created by SystemAdmin publicChannel6 := th.CreateChannelWithClient(th.SystemAdminClient, model.CHANNEL_OPEN) privateChannel7 := th.CreateChannelWithClient(th.SystemAdminClient, model.CHANNEL_PRIVATE) - th.App.AddUserToChannel(user, publicChannel6) - th.App.AddUserToChannel(user, privateChannel7) - th.App.AddUserToChannel(user, privateChannel7) + th.App.AddUserToChannel(user, publicChannel6, false) + th.App.AddUserToChannel(user, privateChannel7, false) + th.App.AddUserToChannel(user, privateChannel7, false) // successful delete by user _, resp := Client.DeleteChannel(publicChannel6.Id) @@ -1594,9 +1595,9 @@ func TestDeleteChannel2(t *testing.T) { // channels created by SystemAdmin publicChannel6 = th.CreateChannelWithClient(th.SystemAdminClient, model.CHANNEL_OPEN) privateChannel7 = th.CreateChannelWithClient(th.SystemAdminClient, model.CHANNEL_PRIVATE) - th.App.AddUserToChannel(user, publicChannel6) - th.App.AddUserToChannel(user, privateChannel7) - th.App.AddUserToChannel(user, privateChannel7) + th.App.AddUserToChannel(user, publicChannel6, false) + th.App.AddUserToChannel(user, privateChannel7, false) + th.App.AddUserToChannel(user, privateChannel7, false) // cannot delete by user _, resp = Client.DeleteChannel(publicChannel6.Id) @@ -2295,7 +2296,7 @@ func TestUpdateChannelRoles(t *testing.T) { channel := th.CreatePublicChannel() // Adds User 2 to the channel, making them a channel member by default. - th.App.AddUserToChannel(th.BasicUser2, channel) + th.App.AddUserToChannel(th.BasicUser2, channel, false) // User 1 promotes User 2 pass, resp := Client.UpdateChannelRoles(channel.Id, th.BasicUser2.Id, ChannelAdmin) @@ -2803,9 +2804,9 @@ func TestRemoveChannelMember(t *testing.T) { t.Run("success", func(t *testing.T) { // Setup the system administrator to listen for websocket events from the channels. th.LinkUserToTeam(th.SystemAdminUser, th.BasicTeam) - _, err := th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel) + _, err := th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel, false) require.Nil(t, err) - _, err = th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel2) + _, err = th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel2, false) require.Nil(t, err) props := map[string]string{} props[model.DESKTOP_NOTIFY_PROP] = model.CHANNEL_NOTIFY_ALL @@ -2847,7 +2848,7 @@ func TestRemoveChannelMember(t *testing.T) { } } - th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel) + th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel, false) _, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser2.Id) CheckNoError(t, resp) @@ -2878,8 +2879,8 @@ func TestRemoveChannelMember(t *testing.T) { // Leave deleted channel th.LoginBasic() deletedChannel := th.CreatePublicChannel() - th.App.AddUserToChannel(th.BasicUser, deletedChannel) - th.App.AddUserToChannel(th.BasicUser2, deletedChannel) + th.App.AddUserToChannel(th.BasicUser, deletedChannel, false) + th.App.AddUserToChannel(th.BasicUser2, deletedChannel, false) deletedChannel.DeleteAt = 1 th.App.UpdateChannel(deletedChannel) @@ -2889,7 +2890,7 @@ func TestRemoveChannelMember(t *testing.T) { th.LoginBasic() private := th.CreatePrivateChannel() - th.App.AddUserToChannel(th.BasicUser2, private) + th.App.AddUserToChannel(th.BasicUser2, private, false) _, resp = Client.RemoveUserFromChannel(private.Id, th.BasicUser2.Id) CheckNoError(t, resp) @@ -2899,7 +2900,7 @@ func TestRemoveChannelMember(t *testing.T) { CheckForbiddenStatus(t, resp) th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - th.App.AddUserToChannel(th.BasicUser, private) + th.App.AddUserToChannel(th.BasicUser, private, false) _, resp = client.RemoveUserFromChannel(private.Id, th.BasicUser.Id) CheckNoError(t, resp) }) @@ -3436,9 +3437,9 @@ func TestChannelMembersMinusGroupMembers(t *testing.T) { channel := th.CreatePrivateChannel() - _, err := th.App.AddChannelMember(user1.Id, channel, "", "") + _, err := th.App.AddChannelMember(user1.Id, channel, app.ChannelMemberOpts{}) require.Nil(t, err) - _, err = th.App.AddChannelMember(user2.Id, channel, "", "") + _, err = th.App.AddChannelMember(user2.Id, channel, app.ChannelMemberOpts{}) require.Nil(t, err) channel.GroupConstrained = model.NewBool(true) diff --git a/api4/post_test.go b/api4/post_test.go index e045b3207d..1ee10fb714 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -540,7 +540,7 @@ func TestCreatePostSendOutOfChannelMentions(t *testing.T) { inChannelUser := th.CreateUser() th.LinkUserToTeam(inChannelUser, th.BasicTeam) - th.App.AddUserToChannel(inChannelUser, th.BasicChannel) + th.App.AddUserToChannel(inChannelUser, th.BasicChannel, false) post1 := &model.Post{ChannelId: th.BasicChannel.Id, Message: "@" + inChannelUser.Username} _, resp := Client.CreatePost(post1) @@ -1997,7 +1997,7 @@ func TestDeletePost(t *testing.T) { func TestDeletePostMessage(t *testing.T) { th := Setup(t).InitBasic() th.LinkUserToTeam(th.SystemAdminUser, th.BasicTeam) - th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel) + th.App.AddUserToChannel(th.SystemAdminUser, th.BasicChannel, false) defer th.TearDown() @@ -2312,8 +2312,8 @@ func TestSearchPostsFromUser(t *testing.T) { th.LoginTeamAdmin() user := th.CreateUser() th.LinkUserToTeam(user, th.BasicTeam) - th.App.AddUserToChannel(user, th.BasicChannel) - th.App.AddUserToChannel(user, th.BasicChannel2) + th.App.AddUserToChannel(user, th.BasicChannel, false) + th.App.AddUserToChannel(user, th.BasicChannel2, false) message := "sgtitlereview with space" _ = th.CreateMessagePost(message) diff --git a/api4/user_test.go b/api4/user_test.go index ee1f6c376d..e637e20063 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -381,7 +381,7 @@ func TestCreateUserWebSocketEvent(t *testing.T) { _, _, err = th.App.AddUserToTeam(th.BasicTeam.Id, guest.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(guest, th.BasicChannel) + _, err = th.App.AddUserToChannel(guest, th.BasicChannel, false) require.Nil(t, err) guestClient := th.CreateClient() @@ -5151,11 +5151,11 @@ func TestGetKnownUsers(t *testing.T) { th.LinkUserToTeam(u3, t2) th.LinkUserToTeam(u4, t3) - th.App.AddUserToChannel(u1, c1) - th.App.AddUserToChannel(u1, c2) - th.App.AddUserToChannel(u2, c1) - th.App.AddUserToChannel(u3, c2) - th.App.AddUserToChannel(u4, c3) + th.App.AddUserToChannel(u1, c1, false) + th.App.AddUserToChannel(u1, c2, false) + th.App.AddUserToChannel(u2, c1, false) + th.App.AddUserToChannel(u3, c2, false) + th.App.AddUserToChannel(u4, c3, false) t.Run("get know users sharing no channels", func(t *testing.T) { _, _ = th.Client.Login(u4.Email, u4.Password) diff --git a/app/app_iface.go b/app/app_iface.go index 27a25b05af..acfed49d46 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -42,12 +42,16 @@ type AppIface interface { ListAutocompleteCommands(teamID string, T i18n.TranslateFunc) ([]*model.Command, *model.AppError) // @openTracingParams teamID, skipSlackParsing CreateCommandPost(post *model.Post, teamID string, response *model.CommandResponse, skipSlackParsing bool) (*model.Post, *model.AppError) + // AddChannelMember adds a user to a channel. It is a wrapper over AddUserToChannel. + AddChannelMember(userID string, channel *model.Channel, opts ChannelMemberOpts) (*model.ChannelMember, *model.AppError) // AddCursorIdsForPostList adds NextPostId and PrevPostId as cursor to the PostList. // The conditional blocks ensure that it sets those cursor IDs immediately as afterPost, beforePost or empty, // and only query to database whenever necessary. AddCursorIdsForPostList(originalList *model.PostList, afterPost, beforePost string, since int64, page, perPage int, collapsedThreads bool) // AddPublicKey will add plugin public key to the config. Overwrites the previous file AddPublicKey(name string, key io.Reader) *model.AppError + // AddUserToChannel adds a user to a given channel. + AddUserToChannel(user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError) // Caller must close the first return value FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) // ChannelMembersMinusGroupMembers returns the set of users in the given channel minus the set of users in the given @@ -359,7 +363,6 @@ type AppIface interface { AcceptLanguage() string AccountMigration() einterfaces.AccountMigrationInterface ActivateMfa(userID, token string) *model.AppError - AddChannelMember(userID string, channel *model.Channel, userRequestorId string, postRootId string) (*model.ChannelMember, *model.AppError) AddConfigListener(listener func(*model.Config, *model.Config)) string AddDirectChannels(teamID string, user *model.User) *model.AppError AddLdapPrivateCertificate(fileData *multipart.FileHeader) *model.AppError @@ -375,7 +378,6 @@ type AppIface interface { AddTeamMemberByInviteId(inviteId, userID string) (*model.TeamMember, *model.AppError) AddTeamMemberByToken(userID, tokenID string) (*model.TeamMember, *model.AppError) AddTeamMembers(teamID string, userIDs []string, userRequestorId string, graceful bool) ([]*model.TeamMemberWithError, *model.AppError) - AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) AddUserToTeam(teamID string, userID string, userRequestorId string) (*model.Team, *model.TeamMember, *model.AppError) AddUserToTeamByInviteId(inviteId string, userID string) (*model.Team, *model.TeamMember, *model.AppError) AddUserToTeamByTeamId(teamID string, user *model.User) *model.AppError diff --git a/app/channel.go b/app/channel.go index 40839e81b9..605d28c96c 100644 --- a/app/channel.go +++ b/app/channel.go @@ -15,7 +15,6 @@ import ( "github.com/mattermost/mattermost-server/v5/shared/i18n" "github.com/mattermost/mattermost-server/v5/shared/mlog" "github.com/mattermost/mattermost-server/v5/store" - "github.com/mattermost/mattermost-server/v5/store/sqlstore" "github.com/mattermost/mattermost-server/v5/utils" ) @@ -1372,24 +1371,23 @@ func (a *App) addUserToChannel(user *model.User, channel *model.Channel) (*model return newMember, nil } -func (a *App) AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) { - // We have to query master here because this is called during LDAP sync from: - // a.createDefaultChannelMemberships -> a.AddTeamMember -> a.AddChannelMember - // So we get a teamMember right after adding a team member which leads to a failure. - // TODO: pass the team member to this method. - teamMember, nErr := a.Srv().Store.Team().GetMember(sqlstore.WithMaster(context.Background()), channel.TeamId, user.Id) - if nErr != nil { - var nfErr *store.ErrNotFound - switch { - case errors.As(nErr, &nfErr): - return nil, model.NewAppError("AddUserToChannel", "app.team.get_member.missing.app_error", nil, nfErr.Error(), http.StatusNotFound) - default: - return nil, model.NewAppError("AddUserToChannel", "app.team.get_member.app_error", nil, nErr.Error(), http.StatusInternalServerError) +// AddUserToChannel adds a user to a given channel. +func (a *App) AddUserToChannel(user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError) { + if !skipTeamMemberIntegrityCheck { + teamMember, nErr := a.Srv().Store.Team().GetMember(context.Background(), channel.TeamId, user.Id) + if nErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(nErr, &nfErr): + return nil, model.NewAppError("AddUserToChannel", "app.team.get_member.missing.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("AddUserToChannel", "app.team.get_member.app_error", nil, nErr.Error(), http.StatusInternalServerError) + } } - } - if teamMember.DeleteAt > 0 { - return nil, model.NewAppError("AddUserToChannel", "api.channel.add_user.to.channel.failed.deleted.app_error", nil, "", http.StatusBadRequest) + if teamMember.DeleteAt > 0 { + return nil, model.NewAppError("AddUserToChannel", "api.channel.add_user.to.channel.failed.deleted.app_error", nil, "", http.StatusBadRequest) + } } newMember, err := a.addUserToChannel(user, channel) @@ -1405,7 +1403,18 @@ func (a *App) AddUserToChannel(user *model.User, channel *model.Channel) (*model return newMember, nil } -func (a *App) AddChannelMember(userID string, channel *model.Channel, userRequestorId string, postRootId string) (*model.ChannelMember, *model.AppError) { +type ChannelMemberOpts struct { + UserRequestorID string + PostRootID string + // SkipTeamMemberIntegrityCheck is used to indicate whether it should be checked + // that a user has already been removed from that team or not. + // This is useful to avoid in scenarios when we just added the team member, + // and thereby know that there is no need to check this. + SkipTeamMemberIntegrityCheck bool +} + +// AddChannelMember adds a user to a channel. It is a wrapper over AddUserToChannel. +func (a *App) AddChannelMember(userID string, channel *model.Channel, opts ChannelMemberOpts) (*model.ChannelMember, *model.AppError) { if member, err := a.Srv().Store.Channel().GetMember(context.Background(), channel.Id, userID); err != nil { var nfErr *store.ErrNotFound if !errors.As(err, &nfErr) { @@ -1423,13 +1432,13 @@ func (a *App) AddChannelMember(userID string, channel *model.Channel, userReques } var userRequestor *model.User - if userRequestorId != "" { - if userRequestor, err = a.GetUser(userRequestorId); err != nil { + if opts.UserRequestorID != "" { + if userRequestor, err = a.GetUser(opts.UserRequestorID); err != nil { return nil, err } } - cm, err := a.AddUserToChannel(user, channel) + cm, err := a.AddUserToChannel(user, channel, opts.SkipTeamMemberIntegrityCheck) if err != nil { return nil, err } @@ -1444,11 +1453,11 @@ func (a *App) AddChannelMember(userID string, channel *model.Channel, userReques }) } - if userRequestorId == "" || userID == userRequestorId { + if opts.UserRequestorID == "" || userID == opts.UserRequestorID { a.postJoinChannelMessage(user, channel) } else { a.Srv().Go(func() { - a.PostAddToChannelMessage(userRequestor, user, channel, postRootId) + a.PostAddToChannelMessage(userRequestor, user, channel, opts.PostRootID) }) } @@ -1938,7 +1947,7 @@ func (a *App) JoinChannel(channel *model.Channel, userID string) *model.AppError return model.NewAppError("JoinChannel", "api.channel.join_channel.permissions.app_error", nil, "", http.StatusBadRequest) } - cm, err := a.AddUserToChannel(user, channel) + cm, err := a.AddUserToChannel(user, channel, false) if err != nil { return err } diff --git a/app/channel_test.go b/app/channel_test.go index d83299509e..0ce6e81af2 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -87,7 +87,7 @@ func TestRemoveAllDeactivatedMembersFromChannel(t *testing.T) { deacivatedUser := th.CreateUser() _, _, err = th.App.AddUserToTeam(team.Id, deacivatedUser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(deacivatedUser, channel) + _, err = th.App.AddUserToChannel(deacivatedUser, channel, false) require.Nil(t, err) channelMembers, err := th.App.GetChannelMembersPage(channel.Id, 0, 10000000) require.Nil(t, err) @@ -127,10 +127,10 @@ func TestMoveChannel(t *testing.T) { _, _, err = th.App.AddUserToTeam(targetTeam.Id, th.BasicUser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(th.BasicUser, channel1) + _, err = th.App.AddUserToChannel(th.BasicUser, channel1, false) require.Nil(t, err) - _, err = th.App.AddUserToChannel(th.BasicUser2, channel1) + _, err = th.App.AddUserToChannel(th.BasicUser2, channel1, false) require.Nil(t, err) err = th.App.MoveChannel(targetTeam, channel1, th.BasicUser) @@ -150,10 +150,10 @@ func TestMoveChannel(t *testing.T) { _, _, err = th.App.AddUserToTeam(sourceTeam.Id, deacivatedUser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(th.BasicUser, channel2) + _, err = th.App.AddUserToChannel(th.BasicUser, channel2, false) require.Nil(t, err) - _, err = th.App.AddUserToChannel(deacivatedUser, channel2) + _, err = th.App.AddUserToChannel(deacivatedUser, channel2, false) require.Nil(t, err) _, err = th.App.UpdateActive(deacivatedUser, false) @@ -241,9 +241,9 @@ func TestRemoveUsersFromChannelNotMemberOfTeam(t *testing.T) { _, _, err = th.App.AddUserToTeam(team.Id, th.BasicUser2.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(th.BasicUser, channel1) + _, err = th.App.AddUserToChannel(th.BasicUser, channel1, false) require.Nil(t, err) - _, err = th.App.AddUserToChannel(th.BasicUser2, channel1) + _, err = th.App.AddUserToChannel(th.BasicUser2, channel1, false) require.Nil(t, err) err = th.App.RemoveUsersFromChannelNotMemberOfTeam(th.SystemAdminUser, channel1, team2) @@ -495,7 +495,7 @@ func TestAddUserToChannelCreatesChannelMemberHistoryRecord(t *testing.T) { channel := th.createChannel(th.BasicTeam, model.CHANNEL_OPEN) - _, err = th.App.AddUserToChannel(user, channel) + _, err = th.App.AddUserToChannel(user, channel, false) require.Nil(t, err, "Failed to add user to channel.") // there should be a ChannelMemberHistory record for the user @@ -581,9 +581,8 @@ func TestAddChannelMemberNoUserRequestor(t *testing.T) { groupUserIds = append(groupUserIds, user.Id) channel := th.createChannel(th.BasicTeam, model.CHANNEL_OPEN) - userRequestorId := "" - postRootId := "" - _, err = th.App.AddChannelMember(user.Id, channel, userRequestorId, postRootId) + + _, err = th.App.AddChannelMember(user.Id, channel, ChannelMemberOpts{}) require.Nil(t, err, "Failed to add user to channel.") // there should be a ChannelMemberHistory record for the user @@ -952,9 +951,7 @@ func TestGetChannelMembersTimezones(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - userRequestorId := "" - postRootId := "" - _, err := th.App.AddChannelMember(th.BasicUser2.Id, th.BasicChannel, userRequestorId, postRootId) + _, err := th.App.AddChannelMember(th.BasicUser2.Id, th.BasicChannel, ChannelMemberOpts{}) require.Nil(t, err, "Failed to add user to channel.") user := th.BasicUser @@ -968,14 +965,14 @@ func TestGetChannelMembersTimezones(t *testing.T) { user3 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""} ruser, _ := th.App.CreateUser(&user3) - th.App.AddUserToChannel(ruser, th.BasicChannel) + th.App.AddUserToChannel(ruser, th.BasicChannel, false) ruser.Timezone["automaticTimezone"] = "NoWhere/Island" th.App.UpdateUser(ruser, false) user4 := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""} ruser, _ = th.App.CreateUser(&user4) - th.App.AddUserToChannel(ruser, th.BasicChannel) + th.App.AddUserToChannel(ruser, th.BasicChannel, false) timezones, err := th.App.GetChannelMembersTimezones(th.BasicChannel.Id) require.Nil(t, err, "Failed to get the timezones for a channel.") @@ -1101,7 +1098,7 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) { _, _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(ruser, th.BasicChannel) + _, err = th.App.AddUserToChannel(ruser, th.BasicChannel, false) require.Nil(t, err) _, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_user") @@ -1115,7 +1112,7 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) { _, _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(ruser, th.BasicChannel) + _, err = th.App.AddUserToChannel(ruser, th.BasicChannel, false) require.Nil(t, err) _, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest") @@ -1129,7 +1126,7 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) { _, _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(ruser, th.BasicChannel) + _, err = th.App.AddUserToChannel(ruser, th.BasicChannel, false) require.Nil(t, err) _, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_user channel_admin") @@ -1143,7 +1140,7 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) { _, _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(ruser, th.BasicChannel) + _, err = th.App.AddUserToChannel(ruser, th.BasicChannel, false) require.Nil(t, err) _, err = th.App.CreateRole(&model.Role{Name: "custom", DisplayName: "custom", Description: "custom"}) @@ -1160,7 +1157,7 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) { _, _, err := th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "") require.Nil(t, err) - _, err = th.App.AddUserToChannel(ruser, th.BasicChannel) + _, err = th.App.AddUserToChannel(ruser, th.BasicChannel, false) require.Nil(t, err) _, err = th.App.UpdateChannelMemberRoles(th.BasicChannel.Id, ruser.Id, "channel_guest channel_user") @@ -1205,9 +1202,9 @@ func TestSearchChannelsForUser(t *testing.T) { }() // add user to test-dev-1 and dev3 - _, err = th.App.AddUserToChannel(th.BasicUser, c1) + _, err = th.App.AddUserToChannel(th.BasicUser, c1, false) require.Nil(t, err) - _, err = th.App.AddUserToChannel(th.BasicUser, c3) + _, err = th.App.AddUserToChannel(th.BasicUser, c3, false) require.Nil(t, err) searchAndCheck := func(t *testing.T, term string, expectedDisplayNames []string) { @@ -1231,7 +1228,7 @@ func TestSearchChannelsForUser(t *testing.T) { }) t.Run("After adding user to test-dev-2, search for dev, the three channels should be returned", func(t *testing.T) { - _, err = th.App.AddUserToChannel(th.BasicUser, c2) + _, err = th.App.AddUserToChannel(th.BasicUser, c2, false) require.Nil(t, err) searchAndCheck(t, "dev", []string{"test-dev-1", "test-dev-2", "dev-3"}) @@ -1332,7 +1329,7 @@ func TestMarkChannelAsUnreadFromPost(t *testing.T) { t.Run("Unread with mentions", func(t *testing.T) { c2 := th.CreateChannel(th.BasicTeam) - _, err := th.App.AddUserToChannel(u2, c2) + _, err := th.App.AddUserToChannel(u2, c2, false) require.Nil(t, err) p4, err := th.App.CreatePost(&model.Post{ @@ -1445,7 +1442,7 @@ func TestAddUserToChannel(t *testing.T) { require.Nil(t, err) // Should allow a bot to be added to a public group synced channel - _, err = th.App.AddUserToChannel(botUser, th.BasicChannel) + _, err = th.App.AddUserToChannel(botUser, th.BasicChannel, false) require.Nil(t, err) // verify user was added as an admin @@ -1466,11 +1463,11 @@ func TestAddUserToChannel(t *testing.T) { require.Nil(t, err) // Should allow a group synced user to be added to a group synced private channel - _, err = th.App.AddUserToChannel(ruser1, privateChannel) + _, err = th.App.AddUserToChannel(ruser1, privateChannel, false) require.Nil(t, err) // Should allow a bot to be added to a private group synced channel - _, err = th.App.AddUserToChannel(botUser, privateChannel) + _, err = th.App.AddUserToChannel(botUser, privateChannel, false) require.Nil(t, err) } @@ -1491,9 +1488,9 @@ func TestRemoveUserFromChannel(t *testing.T) { privateChannel := th.CreatePrivateChannel(th.BasicTeam) - _, err := th.App.AddUserToChannel(ruser, privateChannel) + _, err := th.App.AddUserToChannel(ruser, privateChannel, false) require.Nil(t, err) - _, err = th.App.AddUserToChannel(botUser, privateChannel) + _, err = th.App.AddUserToChannel(botUser, privateChannel, false) require.Nil(t, err) group := th.CreateGroup() diff --git a/app/helper_test.go b/app/helper_test.go index 9954adc898..28ee5c06df 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -422,7 +422,7 @@ func (th *TestHelper) RemoveUserFromTeam(user *model.User, team *model.Team) { func (th *TestHelper) AddUserToChannel(user *model.User, channel *model.Channel) *model.ChannelMember { utils.DisableDebugLogForTest() - member, err := th.App.AddUserToChannel(user, channel) + member, err := th.App.AddUserToChannel(user, channel, false) if err != nil { panic(err) } diff --git a/app/notification_test.go b/app/notification_test.go index 668820ff00..d373a35232 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -19,7 +19,7 @@ func TestSendNotifications(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel) + th.App.AddUserToChannel(th.BasicUser2, th.BasicChannel, false) post1, appErr := th.App.CreatePostMissingChannel(&model.Post{ UserId: th.BasicUser.Id, @@ -126,7 +126,7 @@ func TestSendNotificationsWithManyUsers(t *testing.T) { for i := 0; i < 10; i++ { user := th.CreateUser() th.LinkUserToTeam(user, th.BasicTeam) - th.App.AddUserToChannel(user, th.BasicChannel) + th.App.AddUserToChannel(user, th.BasicChannel, false) users = append(users, user) } @@ -217,9 +217,9 @@ func TestFilterOutOfChannelMentions(t *testing.T) { th.LinkUserToTeam(user3, th.BasicTeam) th.LinkUserToTeam(user4, th.BasicTeam) th.LinkUserToTeam(guest, th.BasicTeam) - th.App.AddUserToChannel(guest, channel) - th.App.AddUserToChannel(user4, guestAndUser4Channel) - th.App.AddUserToChannel(guest, guestAndUser4Channel) + th.App.AddUserToChannel(guest, channel, false) + th.App.AddUserToChannel(user4, guestAndUser4Channel, false) + th.App.AddUserToChannel(guest, guestAndUser4Channel, false) t.Run("should return users not in the channel", func(t *testing.T) { post := &model.Post{} @@ -2420,13 +2420,13 @@ func TestInsertGroupMentions(t *testing.T) { groupChannelMember := th.CreateUser() th.LinkUserToTeam(groupChannelMember, team) - th.App.AddUserToChannel(groupChannelMember, channel) + th.App.AddUserToChannel(groupChannelMember, channel, false) _, err = th.App.UpsertGroupMember(group.Id, groupChannelMember.Id) require.Nil(t, err) nonGroupChannelMember := th.CreateUser() th.LinkUserToTeam(nonGroupChannelMember, team) - th.App.AddUserToChannel(nonGroupChannelMember, channel) + th.App.AddUserToChannel(nonGroupChannelMember, channel, false) nonChannelGroupMember := th.CreateUser() th.LinkUserToTeam(nonChannelGroupMember, team) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index f9473d0262..79e6730653 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -94,7 +94,7 @@ func (a *OpenTracingAppLayer) ActivateMfa(userID string, token string) *model.Ap return resultVar0 } -func (a *OpenTracingAppLayer) AddChannelMember(userID string, channel *model.Channel, userRequestorId string, postRootId string) (*model.ChannelMember, *model.AppError) { +func (a *OpenTracingAppLayer) AddChannelMember(userID string, channel *model.Channel, opts app.ChannelMemberOpts) (*model.ChannelMember, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.AddChannelMember") @@ -106,7 +106,7 @@ func (a *OpenTracingAppLayer) AddChannelMember(userID string, channel *model.Cha }() defer span.Finish() - resultVar0, resultVar1 := a.app.AddChannelMember(userID, channel, userRequestorId, postRootId) + resultVar0, resultVar1 := a.app.AddChannelMember(userID, channel, opts) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -457,7 +457,7 @@ func (a *OpenTracingAppLayer) AddTeamMembers(teamID string, userIDs []string, us return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) { +func (a *OpenTracingAppLayer) AddUserToChannel(user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.AddUserToChannel") @@ -469,7 +469,7 @@ func (a *OpenTracingAppLayer) AddUserToChannel(user *model.User, channel *model. }() defer span.Finish() - resultVar0, resultVar1 := a.app.AddUserToChannel(user, channel) + resultVar0, resultVar1 := a.app.AddUserToChannel(user, channel, skipTeamMemberIntegrityCheck) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/app/plugin_api.go b/app/plugin_api.go index 503cd3fd95..53624f61a1 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -478,27 +478,27 @@ func (api *PluginAPI) SearchPostsInTeamForUser(teamID string, userID string, sea } func (api *PluginAPI) AddChannelMember(channelID, userID string) (*model.ChannelMember, *model.AppError) { - // For now, don't allow overriding these via the plugin API. - userRequestorId := "" - postRootId := "" - channel, err := api.GetChannel(channelID) if err != nil { return nil, err } - return api.app.AddChannelMember(userID, channel, userRequestorId, postRootId) + return api.app.AddChannelMember(userID, channel, ChannelMemberOpts{ + // For now, don't allow overriding these via the plugin API. + UserRequestorID: "", + PostRootID: "", + }) } -func (api *PluginAPI) AddUserToChannel(channelID, userID, asUserId string) (*model.ChannelMember, *model.AppError) { - postRootId := "" - +func (api *PluginAPI) AddUserToChannel(channelID, userID, asUserID string) (*model.ChannelMember, *model.AppError) { channel, err := api.GetChannel(channelID) if err != nil { return nil, err } - return api.app.AddChannelMember(userID, channel, asUserId, postRootId) + return api.app.AddChannelMember(userID, channel, ChannelMemberOpts{ + UserRequestorID: asUserID, + }) } func (api *PluginAPI) GetChannelMember(channelID, userID string) (*model.ChannelMember, *model.AppError) { diff --git a/app/post_test.go b/app/post_test.go index 6832f790a5..666da41922 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -338,7 +338,7 @@ func TestPostReplyToPostWhereRootPosterLeftChannel(t *testing.T) { userNotInChannel := th.BasicUser rootPost := th.BasicPost - _, err := th.App.AddUserToChannel(userInChannel, channel) + _, err := th.App.AddUserToChannel(userInChannel, channel, false) require.Nil(t, err) err = th.App.RemoveUserFromChannel(userNotInChannel.Id, "", channel) @@ -421,7 +421,7 @@ func TestPostChannelMentions(t *testing.T) { require.Nil(t, err) defer th.App.PermanentDeleteChannel(channelToMention) - _, err = th.App.AddUserToChannel(user, channel) + _, err = th.App.AddUserToChannel(user, channel, false) require.Nil(t, err) post := &model.Post{ diff --git a/app/slashcommands/command_invite.go b/app/slashcommands/command_invite.go index c7c289caed..b8f54263ea 100644 --- a/app/slashcommands/command_invite.go +++ b/app/slashcommands/command_invite.go @@ -140,7 +140,9 @@ func (*InviteProvider) DoCommand(a *app.App, args *model.CommandArgs, message st } } - if _, err := a.AddChannelMember(userProfile.Id, channelToJoin, args.UserId, ""); err != nil { + if _, err := a.AddChannelMember(userProfile.Id, channelToJoin, app.ChannelMemberOpts{ + UserRequestorID: args.UserId, + }); err != nil { var text string if err.Id == "api.channel.add_members.user_denied" { text = args.T("api.command_invite.group_constrained_user_denied") diff --git a/app/slashcommands/command_invite_test.go b/app/slashcommands/command_invite_test.go index e451d178e0..d3f84cc7cd 100644 --- a/app/slashcommands/command_invite_test.go +++ b/app/slashcommands/command_invite_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/v5/app" "github.com/mattermost/mattermost-server/v5/model" ) @@ -72,7 +73,7 @@ func TestInviteProvider(t *testing.T) { deactivatedUserPublicChannel := "@" + deactivatedUser.Username + " ~" + channel.Name groupChannel := th.createChannel(th.BasicTeam, model.CHANNEL_PRIVATE) - _, err = th.App.AddChannelMember(th.BasicUser.Id, groupChannel, "", "") + _, err = th.App.AddChannelMember(th.BasicUser.Id, groupChannel, app.ChannelMemberOpts{}) require.Nil(t, err) groupChannel.GroupConstrained = model.NewBool(true) groupChannel, _ = th.App.UpdateChannel(groupChannel) diff --git a/app/slashcommands/command_leave_test.go b/app/slashcommands/command_leave_test.go index a71f8d5326..54cd0e48d1 100644 --- a/app/slashcommands/command_leave_test.go +++ b/app/slashcommands/command_leave_test.go @@ -41,11 +41,11 @@ func TestLeaveProviderDoCommand(t *testing.T) { guest := th.createGuest() th.App.AddUserToTeam(th.BasicTeam.Id, th.BasicUser.Id, th.BasicUser.Id) - th.App.AddUserToChannel(th.BasicUser, publicChannel) - th.App.AddUserToChannel(th.BasicUser, privateChannel) + th.App.AddUserToChannel(th.BasicUser, publicChannel, false) + th.App.AddUserToChannel(th.BasicUser, privateChannel, false) th.App.AddUserToTeam(th.BasicTeam.Id, guest.Id, guest.Id) - th.App.AddUserToChannel(guest, publicChannel) - th.App.AddUserToChannel(guest, defaultChannel) + th.App.AddUserToChannel(guest, publicChannel, false) + th.App.AddUserToChannel(guest, defaultChannel, false) t.Run("Should error when no Channel ID in args", func(t *testing.T) { args := &model.CommandArgs{ diff --git a/app/slashcommands/command_remove_test.go b/app/slashcommands/command_remove_test.go index eeb68bfd8c..1e8a68c19d 100644 --- a/app/slashcommands/command_remove_test.go +++ b/app/slashcommands/command_remove_test.go @@ -35,8 +35,8 @@ func TestRemoveProviderDoCommand(t *testing.T) { targetUser := th.createUser() th.App.AddUserToTeam(th.BasicTeam.Id, targetUser.Id, targetUser.Id) - th.App.AddUserToChannel(targetUser, publicChannel) - th.App.AddUserToChannel(targetUser, privateChannel) + th.App.AddUserToChannel(targetUser, publicChannel, false) + th.App.AddUserToChannel(targetUser, privateChannel, false) // Try a public channel *without* permission. args := &model.CommandArgs{ @@ -49,7 +49,7 @@ func TestRemoveProviderDoCommand(t *testing.T) { assert.Equal(t, "api.command_remove.permission.app_error", actual) // Try a public channel *with* permission. - th.App.AddUserToChannel(th.BasicUser, publicChannel) + th.App.AddUserToChannel(th.BasicUser, publicChannel, false) args = &model.CommandArgs{ T: func(s string, args ...interface{}) string { return s }, ChannelId: publicChannel.Id, @@ -70,7 +70,7 @@ func TestRemoveProviderDoCommand(t *testing.T) { assert.Equal(t, "api.command_remove.permission.app_error", actual) // Try a private channel *with* permission. - th.App.AddUserToChannel(th.BasicUser, privateChannel) + th.App.AddUserToChannel(th.BasicUser, privateChannel, false) args = &model.CommandArgs{ T: func(s string, args ...interface{}) string { return s }, ChannelId: privateChannel.Id, @@ -110,7 +110,7 @@ func TestRemoveProviderDoCommand(t *testing.T) { // Try a public channel with a deactivated user. deactivatedUser := th.createUser() th.App.AddUserToTeam(th.BasicTeam.Id, deactivatedUser.Id, deactivatedUser.Id) - th.App.AddUserToChannel(deactivatedUser, publicChannel) + th.App.AddUserToChannel(deactivatedUser, publicChannel, false) th.App.UpdateActive(deactivatedUser, false) args = &model.CommandArgs{ diff --git a/app/slashcommands/helper_test.go b/app/slashcommands/helper_test.go index 9b0f6afc8b..404488776c 100644 --- a/app/slashcommands/helper_test.go +++ b/app/slashcommands/helper_test.go @@ -358,7 +358,7 @@ func (th *TestHelper) linkUserToTeam(user *model.User, team *model.Team) { func (th *TestHelper) addUserToChannel(user *model.User, channel *model.Channel) *model.ChannelMember { utils.DisableDebugLogForTest() - member, err := th.App.AddUserToChannel(user, channel) + member, err := th.App.AddUserToChannel(user, channel, false) if err != nil { panic(err) } diff --git a/app/syncables.go b/app/syncables.go index e3090930e3..2cda472959 100644 --- a/app/syncables.go +++ b/app/syncables.go @@ -52,7 +52,9 @@ func (a *App) createDefaultChannelMemberships(since int64, channelID *string) er ) } - _, err = a.AddChannelMember(userChannel.UserID, channel, "", "") + _, err = a.AddChannelMember(userChannel.UserID, channel, ChannelMemberOpts{ + SkipTeamMemberIntegrityCheck: true, + }) if err != nil { if err.Id == "api.channel.add_user.to.channel.failed.deleted.app_error" { a.Log().Info("Not adding user to channel because they have already left the team", diff --git a/app/syncables_test.go b/app/syncables_test.go index 7fee2bcece..fc9be6bdeb 100644 --- a/app/syncables_test.go +++ b/app/syncables_test.go @@ -262,7 +262,7 @@ func TestCreateDefaultMemberships(t *testing.T) { } // Ensure members are in channel - _, err = th.App.AddChannelMember(scientist1.Id, experimentsChannel, "", "") + _, err = th.App.AddChannelMember(scientist1.Id, experimentsChannel, ChannelMemberOpts{}) if err != nil { t.Errorf("unable to add user to channel: %s", err.Error()) } @@ -272,7 +272,7 @@ func TestCreateDefaultMemberships(t *testing.T) { if err != nil { t.Errorf("unable to add user to team: %s", err.Error()) } - _, err = th.App.AddChannelMember(singer1.Id, experimentsChannel, "", "") + _, err = th.App.AddChannelMember(singer1.Id, experimentsChannel, ChannelMemberOpts{}) if err != nil { t.Errorf("unable to add user to channel: %s", err.Error()) } @@ -388,7 +388,7 @@ func TestDeleteGroupMemberships(t *testing.T) { _, err = th.App.AddTeamMember(th.BasicTeam.Id, userID) require.Nil(t, err) - _, err = th.App.AddChannelMember(userID, th.BasicChannel, "", "") + _, err = th.App.AddChannelMember(userID, th.BasicChannel, ChannelMemberOpts{}) require.Nil(t, err) } diff --git a/app/team.go b/app/team.go index 38bb37e42a..05b35bf12b 100644 --- a/app/team.go +++ b/app/team.go @@ -623,7 +623,7 @@ func (a *App) AddUserToTeamByToken(userID string, tokenID string) (*model.Team, } for _, channel := range channels { - _, err := a.AddUserToChannel(user, channel) + _, err := a.AddUserToChannel(user, channel, false) if err != nil { mlog.Warn("Error adding user to channel", mlog.Err(err)) } diff --git a/app/user.go b/app/user.go index fd1626d311..ca97a2b3aa 100644 --- a/app/user.go +++ b/app/user.go @@ -104,7 +104,7 @@ func (a *App) CreateUserWithToken(user *model.User, token *model.Token) (*model. if token.Type == TokenTypeGuestInvitation { for _, channel := range channels { - _, err := a.AddChannelMember(ruser.Id, channel, "", "") + _, err := a.AddChannelMember(ruser.Id, channel, ChannelMemberOpts{}) if err != nil { mlog.Warn("Failed to add channel member", mlog.Err(err)) } diff --git a/app/user_test.go b/app/user_test.go index e7b6ac37c5..806e45b741 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -993,9 +993,9 @@ func TestGetViewUsersRestrictions(t *testing.T) { team2townsquare, err := th.App.GetChannelByName("town-square", team2.Id, false) require.Nil(t, err) - th.App.AddUserToChannel(user1, team1channel1) - th.App.AddUserToChannel(user1, team1channel2) - th.App.AddUserToChannel(user1, team2channel1) + th.App.AddUserToChannel(user1, team1channel1, false) + th.App.AddUserToChannel(user1, team1channel2, false) + th.App.AddUserToChannel(user1, team2channel1, false) addPermission := func(role *model.Role, permission string) *model.AppError { newPermissions := append(role.Permissions, permission) diff --git a/cmd/mattermost/commands/channel.go b/cmd/mattermost/commands/channel.go index f0d20ef50e..191383d4b6 100644 --- a/cmd/mattermost/commands/channel.go +++ b/cmd/mattermost/commands/channel.go @@ -311,7 +311,7 @@ func addUserToChannel(a *app.App, channel *model.Channel, user *model.User, user CommandPrintErrorln("Can't find user '" + userArg + "'") return } - if _, err := a.AddUserToChannel(user, channel); err != nil { + if _, err := a.AddUserToChannel(user, channel, false); err != nil { CommandPrintErrorln("Unable to add '" + userArg + "' from " + channel.Name + ". Error: " + err.Error()) return } diff --git a/migrations/helper_test.go b/migrations/helper_test.go index 4c439c8b30..4aad6ae296 100644 --- a/migrations/helper_test.go +++ b/migrations/helper_test.go @@ -217,7 +217,7 @@ func (th *TestHelper) LinkUserToTeam(user *model.User, team *model.Team) { func (th *TestHelper) AddUserToChannel(user *model.User, channel *model.Channel) *model.ChannelMember { utils.DisableDebugLogForTest() - member, err := th.App.AddUserToChannel(user, channel) + member, err := th.App.AddUserToChannel(user, channel, false) if err != nil { panic(err) } diff --git a/services/sharedchannel/mock_AppIface_test.go b/services/sharedchannel/mock_AppIface_test.go index 645a96e2d5..96e96c81e6 100644 --- a/services/sharedchannel/mock_AppIface_test.go +++ b/services/sharedchannel/mock_AppIface_test.go @@ -16,13 +16,13 @@ type MockAppIface struct { mock.Mock } -// AddUserToChannel provides a mock function with given fields: user, channel -func (_m *MockAppIface) AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) { - ret := _m.Called(user, channel) +// AddUserToChannel provides a mock function with given fields: user, channel, skipTeamMemberIntegrityCheck +func (_m *MockAppIface) AddUserToChannel(user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError) { + ret := _m.Called(user, channel, skipTeamMemberIntegrityCheck) var r0 *model.ChannelMember - if rf, ok := ret.Get(0).(func(*model.User, *model.Channel) *model.ChannelMember); ok { - r0 = rf(user, channel) + if rf, ok := ret.Get(0).(func(*model.User, *model.Channel, bool) *model.ChannelMember); ok { + r0 = rf(user, channel, skipTeamMemberIntegrityCheck) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.ChannelMember) @@ -30,8 +30,8 @@ func (_m *MockAppIface) AddUserToChannel(user *model.User, channel *model.Channe } var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.User, *model.Channel) *model.AppError); ok { - r1 = rf(user, channel) + if rf, ok := ret.Get(1).(func(*model.User, *model.Channel, bool) *model.AppError); ok { + r1 = rf(user, channel, skipTeamMemberIntegrityCheck) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError) diff --git a/services/sharedchannel/service.go b/services/sharedchannel/service.go index 9bb02edddb..ebf7dfcae2 100644 --- a/services/sharedchannel/service.go +++ b/services/sharedchannel/service.go @@ -42,7 +42,7 @@ type AppIface interface { SendEphemeralPost(userId string, post *model.Post) *model.Post CreateChannelWithUser(channel *model.Channel, userId string) (*model.Channel, *model.AppError) GetOrCreateDirectChannel(userId, otherUserId string, channelOptions ...model.ChannelOption) (*model.Channel, *model.AppError) - AddUserToChannel(user *model.User, channel *model.Channel) (*model.ChannelMember, *model.AppError) + AddUserToChannel(user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError) AddUserToTeamByTeamId(teamId string, user *model.User) *model.AppError PermanentDeleteChannel(channel *model.Channel) *model.AppError CreatePost(post *model.Post, channel *model.Channel, triggerWebhooks bool, setOnline bool) (savedPost *model.Post, err *model.AppError) diff --git a/services/sharedchannel/sync_recv.go b/services/sharedchannel/sync_recv.go index 8e837849be..b1007cb8f9 100644 --- a/services/sharedchannel/sync_recv.go +++ b/services/sharedchannel/sync_recv.go @@ -222,7 +222,7 @@ func (scs *Service) upsertSyncUser(user *model.User, channel *model.Channel, rc } // add user to channel - if _, err := scs.app.AddUserToChannel(userSaved, channel); err != nil { + if _, err := scs.app.AddUserToChannel(userSaved, channel, false); err != nil { return nil, fmt.Errorf("error adding sync user to ChannelMembers: %w", err) } return userSaved, nil diff --git a/services/slackimport/slackimport.go b/services/slackimport/slackimport.go index 2287ce6002..13a5ff886f 100644 --- a/services/slackimport/slackimport.go +++ b/services/slackimport/slackimport.go @@ -83,7 +83,7 @@ type slackComment struct { // Actions provides the actions that needs to be used for import slack data type Actions struct { UpdateActive func(*model.User, bool) (*model.User, *model.AppError) - AddUserToChannel func(*model.User, *model.Channel) (*model.ChannelMember, *model.AppError) + AddUserToChannel func(*model.User, *model.Channel, bool) (*model.ChannelMember, *model.AppError) JoinUserToTeam func(*model.Team, *model.User, string) (*model.TeamMember, *model.AppError) CreateDirectChannel func(string, string, ...model.ChannelOption) (*model.Channel, *model.AppError) CreateGroupChannel func([]string) (*model.Channel, *model.AppError) @@ -535,7 +535,7 @@ func (si *SlackImporter) addSlackUsersToChannel(members []string, users map[stri log.WriteString(i18n.T("api.slackimport.slack_add_channels.failed_to_add_user", map[string]interface{}{"Username": "?"})) continue } - if _, err := si.actions.AddUserToChannel(user, channel); err != nil { + if _, err := si.actions.AddUserToChannel(user, channel, false); err != nil { log.WriteString(i18n.T("api.slackimport.slack_add_channels.failed_to_add_user", map[string]interface{}{"Username": user.Username})) } }