From 5a3fed4534d8c8abda2b1c30160c5eda8bb69269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Villablanca=20V=C3=A1squez?= Date: Mon, 17 Jun 2019 10:27:19 -0400 Subject: [PATCH] Channel.GetMoreChannels to sync by default (#11207) * Channel.GetMoreChannels to sync by default * var renamed data -> channels --- app/channel.go | 6 +-- store/sqlstore/channel_store.go | 69 +++++++++++++-------------- store/store.go | 2 +- store/storetest/channel_store.go | 33 +++++++------ store/storetest/mocks/ChannelStore.go | 19 ++++++-- 5 files changed, 65 insertions(+), 64 deletions(-) diff --git a/app/channel.go b/app/channel.go index 840b347b20..279606d595 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1265,11 +1265,7 @@ func (a *App) GetDeletedChannels(teamId string, offset int, limit int) (*model.C } func (a *App) GetChannelsUserNotIn(teamId string, userId string, offset int, limit int) (*model.ChannelList, *model.AppError) { - result := <-a.Srv.Store.Channel().GetMoreChannels(teamId, userId, offset, limit) - if result.Err != nil { - return nil, result.Err - } - return result.Data.(*model.ChannelList), nil + return a.Srv.Store.Channel().GetMoreChannels(teamId, userId, offset, limit) } func (a *App) GetPublicChannelsByIdsForTeam(teamId string, channelIds []string) (*model.ChannelList, *model.AppError) { diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index d521dc7292..7026f3b6ad 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -993,49 +993,46 @@ func (s SqlChannelStore) GetAllChannels(offset int, limit int, opts store.Channe }) } -func (s SqlChannelStore) GetMoreChannels(teamId string, userId string, offset int, limit int) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - data := &model.ChannelList{} - _, err := s.GetReplica().Select(data, ` +func (s SqlChannelStore) GetMoreChannels(teamId string, userId string, offset int, limit int) (*model.ChannelList, *model.AppError) { + channels := &model.ChannelList{} + _, err := s.GetReplica().Select(channels, ` + SELECT + Channels.* + FROM + Channels + JOIN + PublicChannels c ON (c.Id = Channels.Id) + WHERE + c.TeamId = :TeamId + AND c.DeleteAt = 0 + AND c.Id NOT IN ( SELECT - Channels.* + c.Id FROM - Channels + PublicChannels c JOIN - PublicChannels c ON (c.Id = Channels.Id) + ChannelMembers cm ON (cm.ChannelId = c.Id) WHERE - c.TeamId = :TeamId + c.TeamId = :TeamId + AND cm.UserId = :UserId AND c.DeleteAt = 0 - AND c.Id NOT IN ( - SELECT - c.Id - FROM - PublicChannels c - JOIN - ChannelMembers cm ON (cm.ChannelId = c.Id) - WHERE - c.TeamId = :TeamId - AND cm.UserId = :UserId - AND c.DeleteAt = 0 - ) - ORDER BY - c.DisplayName - LIMIT :Limit - OFFSET :Offset + ) + ORDER BY + c.DisplayName + LIMIT :Limit + OFFSET :Offset `, map[string]interface{}{ - "TeamId": teamId, - "UserId": userId, - "Limit": limit, - "Offset": offset, - }) - - if err != nil { - result.Err = model.NewAppError("SqlChannelStore.GetMoreChannels", "store.sql_channel.get_more_channels.get.app_error", nil, "teamId="+teamId+", userId="+userId+", err="+err.Error(), http.StatusInternalServerError) - return - } - - result.Data = data + "TeamId": teamId, + "UserId": userId, + "Limit": limit, + "Offset": offset, }) + + if err != nil { + return nil, model.NewAppError("SqlChannelStore.GetMoreChannels", "store.sql_channel.get_more_channels.get.app_error", nil, "teamId="+teamId+", userId="+userId+", err="+err.Error(), http.StatusInternalServerError) + } + + return channels, nil } func (s SqlChannelStore) GetPublicChannelsForTeam(teamId string, offset int, limit int) store.StoreChannel { diff --git a/store/store.go b/store/store.go index 75a21f94dd..04e4791a65 100644 --- a/store/store.go +++ b/store/store.go @@ -149,7 +149,7 @@ type ChannelStore interface { GetDeleted(team_id string, offset int, limit int) StoreChannel GetChannels(teamId string, userId string, includeDeleted bool) StoreChannel GetAllChannels(page, perPage int, opts ChannelSearchOpts) StoreChannel - GetMoreChannels(teamId string, userId string, offset int, limit int) StoreChannel + GetMoreChannels(teamId string, userId string, offset int, limit int) (*model.ChannelList, *model.AppError) GetPublicChannelsForTeam(teamId string, offset int, limit int) StoreChannel GetPublicChannelsByIdsForTeam(teamId string, channelIds []string) StoreChannel GetChannelCounts(teamId string, userId string) StoreChannel diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 8cdfbc0274..0d51ece824 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -610,7 +610,7 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { m2.NotifyProps = model.GetDefaultChannelNotifyProps() store.Must(ss.Channel().SaveMember(&m2)) - if err := ss.Channel().Delete(o1.Id, model.GetMillis()); err != nil { + if err = ss.Channel().Delete(o1.Id, model.GetMillis()); err != nil { t.Fatal(err) } @@ -618,7 +618,7 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { t.Fatal("should have been deleted") } - if err := ss.Channel().Delete(o3.Id, model.GetMillis()); err != nil { + if err = ss.Channel().Delete(o3.Id, model.GetMillis()); err != nil { t.Fatal(err) } @@ -630,9 +630,8 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { t.Fatal("invalid number of channels") } - cresult = <-ss.Channel().GetMoreChannels(o1.TeamId, m1.UserId, 0, 100) - require.Nil(t, cresult.Err) - list = cresult.Data.(*model.ChannelList) + list, err = ss.Channel().GetMoreChannels(o1.TeamId, m1.UserId, 0, 100) + require.Nil(t, err) if len(*list) != 1 { t.Fatal("invalid number of channels") @@ -1272,9 +1271,9 @@ func testChannelStoreGetMoreChannels(t *testing.T, ss store.Store) { })) t.Run("only o3 listed in more channels", func(t *testing.T) { - result := <-ss.Channel().GetMoreChannels(teamId, userId, 0, 100) - require.Nil(t, result.Err) - require.Equal(t, &model.ChannelList{&o3}, result.Data.(*model.ChannelList)) + list, channelErr := ss.Channel().GetMoreChannels(teamId, userId, 0, 100) + require.Nil(t, channelErr) + require.Equal(t, &model.ChannelList{&o3}, list) }) // o6 is another channel on the team to which the user does not belong, and would thus @@ -1303,21 +1302,21 @@ func testChannelStoreGetMoreChannels(t *testing.T, ss store.Store) { require.Nil(t, err, "channel should have been deleted") t.Run("both o3 and o6 listed in more channels", func(t *testing.T) { - result := <-ss.Channel().GetMoreChannels(teamId, userId, 0, 100) - require.Nil(t, result.Err) - require.Equal(t, &model.ChannelList{&o3, &o6}, result.Data.(*model.ChannelList)) + list, err := ss.Channel().GetMoreChannels(teamId, userId, 0, 100) + require.Nil(t, err) + require.Equal(t, &model.ChannelList{&o3, &o6}, list) }) t.Run("only o3 listed in more channels with offset 0, limit 1", func(t *testing.T) { - result := <-ss.Channel().GetMoreChannels(teamId, userId, 0, 1) - require.Nil(t, result.Err) - require.Equal(t, &model.ChannelList{&o3}, result.Data.(*model.ChannelList)) + list, err := ss.Channel().GetMoreChannels(teamId, userId, 0, 1) + require.Nil(t, err) + require.Equal(t, &model.ChannelList{&o3}, list) }) t.Run("only o6 listed in more channels with offset 1, limit 1", func(t *testing.T) { - result := <-ss.Channel().GetMoreChannels(teamId, userId, 1, 1) - require.Nil(t, result.Err) - require.Equal(t, &model.ChannelList{&o6}, result.Data.(*model.ChannelList)) + list, err := ss.Channel().GetMoreChannels(teamId, userId, 1, 1) + require.Nil(t, err) + require.Equal(t, &model.ChannelList{&o6}, list) }) t.Run("verify analytics for open channels", func(t *testing.T) { diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 9035087b01..1de400f9e8 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -661,19 +661,28 @@ func (_m *ChannelStore) GetMembersForUserWithPagination(teamId string, userId st } // GetMoreChannels provides a mock function with given fields: teamId, userId, offset, limit -func (_m *ChannelStore) GetMoreChannels(teamId string, userId string, offset int, limit int) store.StoreChannel { +func (_m *ChannelStore) GetMoreChannels(teamId string, userId string, offset int, limit int) (*model.ChannelList, *model.AppError) { ret := _m.Called(teamId, userId, offset, limit) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, string, int, int) store.StoreChannel); ok { + var r0 *model.ChannelList + if rf, ok := ret.Get(0).(func(string, string, int, int) *model.ChannelList); ok { r0 = rf(teamId, userId, offset, limit) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.ChannelList) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, string, int, int) *model.AppError); ok { + r1 = rf(teamId, userId, offset, limit) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetPinnedPosts provides a mock function with given fields: channelId