From 32616a989bf26c9ef5115cbbd24f70832dbc5cb3 Mon Sep 17 00:00:00 2001 From: Allan Guwatudde Date: Tue, 9 Jul 2019 01:40:02 +0300 Subject: [PATCH] [MM-16772] Migrate Team.RemoveMember to Sync by default (#11565) * Migrate Team.RemoveMember to Sync by default * Fixed typo in user_store.go * Resolved merge conflicts in storetest/user_store.go --- store/sqlstore/team_store.go | 13 ++++++------- store/store.go | 2 +- store/storetest/group_store.go | 24 ++++++++++++------------ store/storetest/mocks/TeamStore.go | 8 ++++---- store/storetest/team_store.go | 14 +++++++------- store/storetest/user_store.go | 11 +++++++---- 6 files changed, 37 insertions(+), 35 deletions(-) diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index ee887efb7e..8a799af194 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -764,13 +764,12 @@ func (s SqlTeamStore) GetChannelUnreadsForTeam(teamId, userId string) ([]*model. return channels, nil } -func (s SqlTeamStore) RemoveMember(teamId string, userId string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - _, err := s.GetMaster().Exec("DELETE FROM TeamMembers WHERE TeamId = :TeamId AND UserId = :UserId", map[string]interface{}{"TeamId": teamId, "UserId": userId}) - if err != nil { - result.Err = model.NewAppError("SqlTeamStore.RemoveMember", "store.sql_team.remove_member.app_error", nil, "team_id="+teamId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) - } - }) +func (s SqlTeamStore) RemoveMember(teamId string, userId string) *model.AppError { + _, err := s.GetMaster().Exec("DELETE FROM TeamMembers WHERE TeamId = :TeamId AND UserId = :UserId", map[string]interface{}{"TeamId": teamId, "UserId": userId}) + if err != nil { + return model.NewAppError("SqlTeamStore.RemoveMember", "store.sql_team.remove_member.app_error", nil, "team_id="+teamId+", user_id="+userId+", "+err.Error(), http.StatusInternalServerError) + } + return nil } func (s SqlTeamStore) RemoveAllMembersByTeam(teamId string) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 26c87647d5..b37df4ef03 100644 --- a/store/store.go +++ b/store/store.go @@ -111,7 +111,7 @@ type TeamStore interface { GetTeamsForUserWithPagination(userId string, page, perPage int) ([]*model.TeamMember, *model.AppError) GetChannelUnreadsForAllTeams(excludeTeamId, userId string) ([]*model.ChannelUnread, *model.AppError) GetChannelUnreadsForTeam(teamId, userId string) ([]*model.ChannelUnread, *model.AppError) - RemoveMember(teamId string, userId string) StoreChannel + RemoveMember(teamId string, userId string) *model.AppError RemoveAllMembersByTeam(teamId string) StoreChannel RemoveAllMembersByUser(userId string) *model.AppError UpdateLastTeamIconUpdate(teamId string, curTime int64) *model.AppError diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index 8eb7d1da94..241dd1c0e9 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -1291,12 +1291,12 @@ func testTeamMemberRemovals(t *testing.T, ss store.Store) { require.Len(t, teamMembers, 3) // add users back to groups - res := <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id) - require.Nil(t, res.Err) - res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id) - require.Nil(t, res.Err) - res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id) - require.Nil(t, res.Err) + res := ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id) + require.Nil(t, res) + res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id) + require.Nil(t, res) + res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id) + require.Nil(t, res) err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id) require.Nil(t, err) err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id) @@ -1366,12 +1366,12 @@ func testChannelMemberRemovals(t *testing.T, ss store.Store) { require.Len(t, channelMembers, 3) // add users back to groups - res := <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id) - require.Nil(t, res.Err) - res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id) - require.Nil(t, res.Err) - res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id) - require.Nil(t, res.Err) + res := ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id) + require.Nil(t, res) + res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserB.Id) + require.Nil(t, res) + res = ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserC.Id) + require.Nil(t, res) err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserA.Id) require.Nil(t, err) err = ss.Channel().RemoveMember(data.ConstrainedChannel.Id, data.UserB.Id) diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index 62eb86c1de..b10faa8a6c 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -696,15 +696,15 @@ func (_m *TeamStore) RemoveAllMembersByUser(userId string) *model.AppError { } // RemoveMember provides a mock function with given fields: teamId, userId -func (_m *TeamStore) RemoveMember(teamId string, userId string) store.StoreChannel { +func (_m *TeamStore) RemoveMember(teamId string, userId string) *model.AppError { ret := _m.Called(teamId, 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(teamId, userId) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } diff --git a/store/storetest/team_store.go b/store/storetest/team_store.go index ac6f17b11b..a3ce242982 100644 --- a/store/storetest/team_store.go +++ b/store/storetest/team_store.go @@ -802,8 +802,8 @@ func testTeamMembers(t *testing.T, ss store.Store) { require.Equal(t, m1.TeamId, ms[0].TeamId) } - if r1 := <-ss.Team().RemoveMember(teamId1, m1.UserId); r1.Err != nil { - t.Fatal(r1.Err) + if err := ss.Team().RemoveMember(teamId1, m1.UserId); err != nil { + t.Fatal(err) } if ms, err := ss.Team().GetMembers(teamId1, 0, 100, nil); err != nil { @@ -872,8 +872,8 @@ func testTeamMembersWithPagination(t *testing.T, ss store.Store) { require.Len(t, ms, 1) require.Equal(t, m1.TeamId, ms[0].TeamId) - r1 = <-ss.Team().RemoveMember(teamId1, m1.UserId) - require.Nil(t, r1.Err) + e := ss.Team().RemoveMember(teamId1, m1.UserId) + require.Nil(t, e) ms, err := ss.Team().GetMembers(teamId1, 0, 100, nil) require.Nil(t, err) @@ -937,7 +937,7 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { }, maxUsersPerTeam)) defer func(userId string) { - <-ss.Team().RemoveMember(team.Id, userId) + ss.Team().RemoveMember(team.Id, userId) }(userIds[i]) } @@ -989,7 +989,7 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { t.Fatal("should've been able to save new member after deleting one", result.Err) } else { defer func(userId string) { - <-ss.Team().RemoveMember(team.Id, userId) + ss.Team().RemoveMember(team.Id, userId) }(newUserId) } @@ -1016,7 +1016,7 @@ func testSaveTeamMemberMaxMembers(t *testing.T, ss store.Store) { t.Fatal("should've been able to save new member after deleting one", result.Err) } else { defer func(userId string) { - <-ss.Team().RemoveMember(team.Id, userId) + ss.Team().RemoveMember(team.Id, userId) }(newUserId2) } } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index dffafd51f3..87b82ce8a8 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -3477,8 +3477,11 @@ func testUserStoreGetProfilesNotInTeam(t *testing.T, ss store.Store) { // Ensure update at timestamp changes time.Sleep(time.Millisecond * 10) - store.Must(ss.Team().RemoveMember(teamId, u1.Id)) - store.Must(ss.Team().RemoveMember(teamId, u2.Id)) + e := ss.Team().RemoveMember(teamId, u1.Id) + require.Nil(t, e) + e = ss.Team().RemoveMember(teamId, u2.Id) + require.Nil(t, e) + u1.UpdateAt, err = ss.User().UpdateUpdateAt(u1.Id) require.Nil(t, err) u2.UpdateAt, err = ss.User().UpdateUpdateAt(u2.Id) @@ -3919,8 +3922,8 @@ func testUserStoreGetTeamGroupUsers(t *testing.T, ss store.Store) { requireNUsers(2) // delete team membership of allowed user - res = <-ss.Team().RemoveMember(team.Id, userGroupA.Id) - require.Nil(t, res.Err) + err = ss.Team().RemoveMember(team.Id, userGroupA.Id) + require.Nil(t, err) // ensure removed allowed member still returned by query requireNUsers(2)