diff --git a/app/channel.go b/app/channel.go index fb0f22137f..eea511302f 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1616,8 +1616,8 @@ func (a *App) removeUserFromChannel(userIdToRemove string, removerUserId string, return err } - if cmresult := <-a.Srv.Store.Channel().RemoveMember(channel.Id, userIdToRemove); cmresult.Err != nil { - return cmresult.Err + if err := a.Srv.Store.Channel().RemoveMember(channel.Id, userIdToRemove); err != nil { + return err } if cmhResult := <-a.Srv.Store.ChannelMemberHistory().LogLeaveEvent(userIdToRemove, channel.Id, model.GetMillis()); cmhResult.Err != nil { return cmhResult.Err diff --git a/app/team.go b/app/team.go index 4f08850357..e1e2786ea6 100644 --- a/app/team.go +++ b/app/team.go @@ -840,8 +840,8 @@ func (a *App) LeaveTeam(team *model.Team, user *model.User, requestorId string) for _, channel := range *channelList { if !channel.IsGroupOrDirect() { a.InvalidateCacheForChannelMembers(channel.Id) - if result := <-a.Srv.Store.Channel().RemoveMember(channel.Id, user.Id); result.Err != nil { - return result.Err + if err := a.Srv.Store.Channel().RemoveMember(channel.Id, user.Id); err != nil { + return err } } } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 799952af70..330a477a06 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1685,13 +1685,12 @@ func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) s }) } -func (s SqlChannelStore) RemoveMember(channelId string, userId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - _, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId}) - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.RemoveMember", "store.sql_channel.remove_member.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) - } - }) +func (s SqlChannelStore) RemoveMember(channelId string, userId string) *model.AppError { + _, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId}) + if err != nil { + return model.NewAppError("SqlChannelStore.RemoveMember", "store.sql_channel.remove_member.app_error", nil, "channel_id="+channelId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) + } + return nil } func (s SqlChannelStore) RemoveAllDeactivatedMembers(channelId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 12340db0a6..1934e9ce73 100644 --- a/store/store.go +++ b/store/store.go @@ -172,7 +172,7 @@ type ChannelStore interface { GetMemberCountFromCache(channelId string) int64 GetMemberCount(channelId string, allowFromCache bool) StoreChannel GetPinnedPosts(channelId string) StoreChannel - RemoveMember(channelId string, userId string) StoreChannel + RemoveMember(channelId string, userId string) *model.AppError PermanentDeleteMembersByUser(userId string) StoreChannel PermanentDeleteMembersByChannel(channelId string) *model.AppError UpdateLastViewedAt(channelIds []string, userId string) StoreChannel diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 7170635e12..166d5114d4 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -910,7 +910,8 @@ func testChannelMemberStore(t *testing.T, ss store.Store) { t.Fatal("should have saved 2 members") } - store.Must(ss.Channel().RemoveMember(o2.ChannelId, o2.UserId)) + err = ss.Channel().RemoveMember(o2.ChannelId, o2.UserId) + require.Nil(t, err) count = (<-ss.Channel().GetMemberCount(o1.ChannelId, false)).Data.(int64) if count != 1 { diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index 397d9979f0..9d0c080a90 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -1324,12 +1324,12 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) { require.Nil(t, res.Err) res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id) require.Nil(t, res.Err) - res = <-ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id) - require.Nil(t, res.Err) - res = <-ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id) - require.Nil(t, res.Err) - res = <-ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserC.Id) - require.Nil(t, res.Err) + err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id) + require.Nil(t, err) + err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id) + require.Nil(t, err) + err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserC.Id) + require.Nil(t, err) } func testChannelMemberRemovals(t *testing.T, ss store.Store) { @@ -1399,12 +1399,12 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) { require.Nil(t, res.Err) res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id) require.Nil(t, res.Err) - res = <-ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id) - require.Nil(t, res.Err) - res = <-ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id) - require.Nil(t, res.Err) - res = <-ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserC.Id) - require.Nil(t, res.Err) + err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id) + require.Nil(t, err) + err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id) + require.Nil(t, err) + err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserC.Id) + require.Nil(t, err) } type removalsData struct { diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 026e053936..edb0874c88 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -996,15 +996,15 @@ func (_m *ChannelStore) RemoveAllDeactivatedMembers(channelId string) store.Stor } // RemoveMember provides a mock function with given fields: channelId, userId -func (_m *ChannelStore) RemoveMember(channelId string, userId string) store.StoreChannel { +func (_m *ChannelStore) RemoveMember(channelId string, userId string) *model.AppError { ret := _m.Called(channelId, userId) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok { r0 = rf(channelId, userId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 6bedc86f92..a494161b5b 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -3825,8 +3825,8 @@ func testUserStoreGetChannelGroupUsers(t *testing.T, ss store.Store) { requireNUsers(2) // delete team membership of allowed user - res = <-ss.Channel().RemoveMember(channel.Id, userGroupA.Id) - require.Nil(t, res.Err) + err = ss.Channel().RemoveMember(channel.Id, userGroupA.Id) + require.Nil(t, err) // ensure removed allowed member still returned by query requireNUsers(2)