Migrate GetChannels method from ChannelStore to return error interface (#14711)

* Migrate GetChannels method from ChannelStore to return error interface

* Fix testing

* Changed error type: ErrInvalidInput -> ErrNotFound

* Added note about error migrations

* Fix en.json

* Fix i18n

Co-authored-by: Agniva De Sarker <agnivade@yahoo.co.in>
Этот коммит содержится в:
Rodrigo Villablanca
2020-06-11 04:32:03 -04:00
коммит произвёл GitHub
родитель 9b0ae49b55
Коммит 41d9c673cf
10 изменённых файлов: 66 добавлений и 39 удалений

Просмотреть файл

@@ -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) {

Просмотреть файл

@@ -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 {

Просмотреть файл

@@ -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."

Просмотреть файл

@@ -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)

Просмотреть файл

@@ -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)

Просмотреть файл

@@ -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()

Просмотреть файл

@@ -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)

Просмотреть файл

@@ -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)

Просмотреть файл

@@ -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

Просмотреть файл

@@ -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)