diff --git a/app/channel.go b/app/channel.go index bd1a8ba685..4efc3219ef 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1551,7 +1551,18 @@ func (a *App) GetChannelByNameForTeamName(channelName, teamName string, includeD } func (a *App) GetChannelsForUser(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { - return a.Srv().Store.Channel().GetChannels(teamId, userId, includeDeleted) + list, err := a.Srv().Store.Channel().GetChannels(teamId, userId, includeDeleted) + if err != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(err, &nfErr): + return nil, model.NewAppError("GetChannelsForUser", "app.channel.get_channels.not_found.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("GetChannelsForUser", "app.channel.get_channels.get.app_error", nil, err.Error(), http.StatusInternalServerError) + } + } + + return list, nil } func (a *App) GetAllChannels(page, perPage int, opts model.ChannelSearchOpts) (*model.ChannelListWithTeamData, *model.AppError) { diff --git a/app/team.go b/app/team.go index 2a705716f8..67ede75502 100644 --- a/app/team.go +++ b/app/team.go @@ -5,6 +5,7 @@ package app import ( "bytes" + "errors" "fmt" "image" "image/png" @@ -988,11 +989,13 @@ func (a *App) LeaveTeam(team *model.Team, user *model.User, requestorId string) var channelList *model.ChannelList - if channelList, err = a.Srv().Store.Channel().GetChannels(team.Id, user.Id, true); err != nil { - if err.Id == "store.sql_channel.get_channels.not_found.app_error" { + var nErr error + if channelList, nErr = a.Srv().Store.Channel().GetChannels(team.Id, user.Id, true); nErr != nil { + var nfErr *store.ErrNotFound + if errors.As(nErr, &nfErr) { channelList = &model.ChannelList{} } else { - return err + return model.NewAppError("LeaveTeam", "app.channel.get_channels.get.app_error", nil, nErr.Error(), http.StatusInternalServerError) } } @@ -1342,7 +1345,7 @@ func (a *App) PermanentDeleteTeam(team *model.Team) *model.AppError { } if channels, err := a.Srv().Store.Channel().GetTeamChannels(team.Id); err != nil { - if err.Id != "store.sql_channel.get_channels.not_found.app_error" { + if err.Id != "app.channel.get_channels.not_found.app_error" { return err } } else { diff --git a/i18n/en.json b/i18n/en.json index c0555a13c5..9ead5a39f4 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3006,6 +3006,14 @@ "id": "app.channel.get_all_channels_count.app_error", "translation": "Unable to count all the channels." }, + { + "id": "app.channel.get_channels.get.app_error", + "translation": "Unable to get the channels." + }, + { + "id": "app.channel.get_channels.not_found.app_error", + "translation": "No channels were found." + }, { "id": "app.channel.get_deleted.existing.app_error", "translation": "Unable to find the existing deleted channel." @@ -6162,14 +6170,6 @@ "id": "store.sql_channel.get_channel_counts.get.app_error", "translation": "Unable to get the channel counts." }, - { - "id": "store.sql_channel.get_channels.get.app_error", - "translation": "Unable to get the channels." - }, - { - "id": "store.sql_channel.get_channels.not_found.app_error", - "translation": "No channels were found." - }, { "id": "store.sql_channel.get_channels_batch_for_indexing.get.app_error", "translation": "Unable to get the channels batch for indexing." diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index 402ad764b1..5d3d431512 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -900,7 +900,7 @@ func (s *OpenTracingLayerChannelStore) GetChannelUnread(channelId string, userId return resultVar0, resultVar1 } -func (s *OpenTracingLayerChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { +func (s *OpenTracingLayerChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.GetChannels") s.Root.Store.SetContext(newCtx) diff --git a/store/searchlayer/post_layer.go b/store/searchlayer/post_layer.go index dc999fc496..bf5decf185 100644 --- a/store/searchlayer/post_layer.go +++ b/store/searchlayer/post_layer.go @@ -4,6 +4,9 @@ package searchlayer import ( + "errors" + "net/http" + "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/services/searchengine" @@ -80,10 +83,17 @@ func (s SearchPostStore) Delete(postId string, date int64, deletedByID string) * func (s SearchPostStore) searchPostsInTeamForUserByEngine(engine searchengine.SearchEngineInterface, paramsList []*model.SearchParams, userId, teamId string, isOrSearch, includeDeletedChannels bool, page, perPage int) (*model.PostSearchResults, *model.AppError) { // We only allow the user to search in channels they are a member of. - userChannels, err := s.rootStore.Channel().GetChannels(teamId, userId, includeDeletedChannels) - if err != nil { - mlog.Error("error getting channel for user", mlog.Err(err)) - return nil, err + userChannels, nErr := s.rootStore.Channel().GetChannels(teamId, userId, includeDeletedChannels) + if nErr != nil { + mlog.Error("error getting channel for user", mlog.Err(nErr)) + var nfErr *store.ErrNotFound + switch { + // TODO: This error key would go away once this store method is migrated to return plain errors + case errors.As(nErr, &nfErr): + return nil, model.NewAppError("searchPostsInTeamForUserByEngine", "app.channel.get_channels.not_found.app_error", nil, nfErr.Error(), http.StatusNotFound) + default: + return nil, model.NewAppError("searchPostsInTeamForUserByEngine", "app.channel.get_channels.get.app_error", nil, nErr.Error(), http.StatusInternalServerError) + } } postIds, matches, err := engine.SearchPosts(userChannels, paramsList, page, perPage) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index b349972f3e..77d720805b 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -912,7 +912,7 @@ func (s SqlChannelStore) PermanentDeleteMembersByChannel(channelId string) *mode return nil } -func (s SqlChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { +func (s SqlChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, error) { query := "SELECT Channels.* FROM Channels, ChannelMembers WHERE Id = ChannelId AND UserId = :UserId AND DeleteAt = 0 AND (TeamId = :TeamId OR TeamId = '') ORDER BY DisplayName" if includeDeleted { query = "SELECT Channels.* FROM Channels, ChannelMembers WHERE Id = ChannelId AND UserId = :UserId AND (TeamId = :TeamId OR TeamId = '') ORDER BY DisplayName" @@ -921,11 +921,11 @@ func (s SqlChannelStore) GetChannels(teamId string, userId string, includeDelete _, err := s.GetReplica().Select(channels, query, map[string]interface{}{"TeamId": teamId, "UserId": userId}) if err != nil { - return nil, model.NewAppError("SqlChannelStore.GetChannels", "store.sql_channel.get_channels.get.app_error", nil, "teamId="+teamId+", userId="+userId+", err="+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get channels with TeamId=%s and UserId=%s", teamId, userId) } if len(*channels) == 0 { - return nil, model.NewAppError("SqlChannelStore.GetChannels", "store.sql_channel.get_channels.not_found.app_error", nil, "teamId="+teamId+", userId="+userId, http.StatusBadRequest) + return nil, store.NewErrNotFound("Channel", "userId="+userId) } return channels, nil @@ -1139,11 +1139,13 @@ func (s SqlChannelStore) GetTeamChannels(teamId string) (*model.ChannelList, *mo _, err := s.GetReplica().Select(data, "SELECT * FROM Channels WHERE TeamId = :TeamId And Type != 'D' ORDER BY DisplayName", map[string]interface{}{"TeamId": teamId}) if err != nil { - return nil, model.NewAppError("SqlChannelStore.GetTeamChannels", "store.sql_channel.get_channels.get.app_error", nil, "teamId="+teamId+", err="+err.Error(), http.StatusInternalServerError) + // TODO: This error key would go away once this store method is migrated to return plain errors + return nil, model.NewAppError("SqlChannelStore.GetTeamChannels", "app.channel.get_channels.get.app_error", nil, "teamId="+teamId+", err="+err.Error(), http.StatusInternalServerError) } if len(*data) == 0 { - return nil, model.NewAppError("SqlChannelStore.GetTeamChannels", "store.sql_channel.get_channels.not_found.app_error", nil, "teamId="+teamId, http.StatusNotFound) + // TODO: This error key would go away once this store method is migrated to return plain errors + return nil, model.NewAppError("SqlChannelStore.GetTeamChannels", "app.channel.get_channels.not_found.app_error", nil, "teamId="+teamId, http.StatusNotFound) } return data, nil @@ -1335,6 +1337,7 @@ func (s SqlChannelStore) SaveMultipleMembers(members []*model.ChannelMember) ([] case errors.As(err, &appErr): // in case we haven't converted to plain error. return nil, appErr default: // last fallback in case it doesn't map to an existing app error. + // TODO: This error key would go away once this store method is migrated to return plain errors return nil, model.NewAppError("CreateDirectChannel", "app.channel.create_direct_channel.internal_error", nil, err.Error(), http.StatusInternalServerError) } } @@ -1713,7 +1716,8 @@ func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCac ChannelMembers.UserId = :UserId`, map[string]interface{}{"UserId": userId}) if err != nil { - return nil, model.NewAppError("SqlChannelStore.GetAllChannelMembersForUser", "store.sql_channel.get_channels.get.app_error", nil, "userId="+userId+", err="+err.Error(), http.StatusInternalServerError) + // TODO: This error key would go away once this store method is migrated to return plain errors + return nil, model.NewAppError("SqlChannelStore.GetAllChannelMembersForUser", "app.channel.get_channels.get.app_error", nil, "userId="+userId+", err="+err.Error(), http.StatusInternalServerError) } ids := data.ToMapStringString() diff --git a/store/store.go b/store/store.go index 5bcea6de29..52df8ccd24 100644 --- a/store/store.go +++ b/store/store.go @@ -148,7 +148,7 @@ type ChannelStore interface { GetByNameIncludeDeleted(team_id string, name string, allowFromCache bool) (*model.Channel, *model.AppError) GetDeletedByName(team_id string, name string) (*model.Channel, *model.AppError) GetDeleted(team_id string, offset int, limit int, userId string) (*model.ChannelList, error) - GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) + GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, error) GetAllChannels(page, perPage int, opts ChannelSearchOpts) (*model.ChannelListWithTeamData, error) GetAllChannelsCount(opts ChannelSearchOpts) (int64, error) GetMoreChannels(teamId string, userId string, offset int, limit int) (*model.ChannelList, *model.AppError) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index a7af4c6509..bc36db4b49 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -609,8 +609,8 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { nErr = ss.Channel().Delete(o3.Id, model.GetMillis()) require.Nil(t, nErr, nErr) - list, err := ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) - require.Nil(t, err) + list, nErr := ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) + require.Nil(t, nErr) require.Len(t, *list, 1, "invalid number of channels") list, err = ss.Channel().GetMoreChannels(o1.TeamId, m1.UserId, 0, 100) @@ -620,9 +620,10 @@ func testChannelStoreDelete(t *testing.T, ss store.Store) { cresult := ss.Channel().PermanentDelete(o2.Id) require.Nil(t, cresult) - list, err = ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) - if assert.NotNil(t, err) { - require.Equal(t, "store.sql_channel.get_channels.not_found.app_error", err.Id) + list, nErr = ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) + if assert.NotNil(t, nErr) { + var nfErr *store.ErrNotFound + require.True(t, errors.As(nErr, &nfErr)) } else { require.Equal(t, &model.ChannelList{}, list) } @@ -3138,8 +3139,8 @@ func testChannelStoreGetChannels(t *testing.T, ss store.Store) { _, err = ss.Channel().SaveMember(&m3) require.Nil(t, err) - list, err := ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) - require.Nil(t, err) + list, nErr := ss.Channel().GetChannels(o1.TeamId, m1.UserId, false) + require.Nil(t, nErr) require.Equal(t, o1.Id, (*list)[0].Id, "missing channel") ids, _ := ss.Channel().GetAllChannelMembersForUser(m1.UserId, false, false) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index a4190f9f43..a158b52f0e 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -560,7 +560,7 @@ func (_m *ChannelStore) GetChannelUnread(channelId string, userId string) (*mode } // GetChannels provides a mock function with given fields: teamId, userId, includeDeleted -func (_m *ChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { +func (_m *ChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, error) { ret := _m.Called(teamId, userId, includeDeleted) var r0 *model.ChannelList @@ -572,13 +572,11 @@ func (_m *ChannelStore) GetChannels(teamId string, userId string, includeDeleted } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, string, bool) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string, string, bool) error); ok { r1 = rf(teamId, userId, includeDeleted) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/timer_layer.go b/store/timer_layer.go index 5685262293..40955bee91 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -840,7 +840,7 @@ func (s *TimerLayerChannelStore) GetChannelUnread(channelId string, userId strin return resultVar0, resultVar1 } -func (s *TimerLayerChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) { +func (s *TimerLayerChannelStore) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, error) { start := timemodule.Now() resultVar0, resultVar1 := s.ChannelStore.GetChannels(teamId, userId, includeDeleted)