From 9ec09c2bcdd3d4759382573762bc47bb03a1a8b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Mon, 23 Nov 2020 09:26:01 +0100 Subject: [PATCH] Remove unneeded Err field in the store result struct (#16322) * Remove unneeded Err field in the store result struct * Fixing references to old errors in the tests --- app/notification.go | 13 +++++++------ app/notification_test.go | 25 ++++++++++++------------- store/store.go | 1 - 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/app/notification.go b/app/notification.go index a21b608e36..02489281b1 100644 --- a/app/notification.go +++ b/app/notification.go @@ -16,6 +16,7 @@ import ( "github.com/mattermost/mattermost-server/v5/store" "github.com/mattermost/mattermost-server/v5/utils" "github.com/mattermost/mattermost-server/v5/utils/markdown" + "github.com/pkg/errors" ) func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *model.Channel, sender *model.User, parentPostList *model.PostList, setOnline bool) ([]string, error) { @@ -43,7 +44,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod gchan = make(chan store.StoreResult, 1) go func() { groupsMap, err := a.getGroupsAllowedForReferenceInChannel(channel, team) - gchan <- store.StoreResult{Data: groupsMap, Err: err} + gchan <- store.StoreResult{Data: groupsMap, NErr: err} close(gchan) }() } @@ -73,8 +74,8 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod groups := make(map[string]*model.Group) if gchan != nil { result = <-gchan - if result.Err != nil { - return nil, result.Err + if result.NErr != nil { + return nil, result.NErr } groups = result.Data.(map[string]*model.Group) } @@ -818,7 +819,7 @@ func (a *App) allowGroupMentions(post *model.Post) bool { } // getGroupsAllowedForReferenceInChannel returns a map of groups allowed for reference in a given channel and team. -func (a *App) getGroupsAllowedForReferenceInChannel(channel *model.Channel, team *model.Team) (map[string]*model.Group, *model.AppError) { +func (a *App) getGroupsAllowedForReferenceInChannel(channel *model.Channel, team *model.Team) (map[string]*model.Group, error) { var err error groupsMap := make(map[string]*model.Group) opts := model.GroupSearchOpts{FilterAllowReference: true} @@ -831,7 +832,7 @@ func (a *App) getGroupsAllowedForReferenceInChannel(channel *model.Channel, team groups, err = a.Srv().Store.Group().GetGroupsByTeam(team.Id, opts) } if err != nil { - return nil, model.NewAppError("getGroupsAllowedForReferenceInChannel", "app.select_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "unable to get groups") } for _, group := range groups { if group.Group.Name != nil { @@ -843,7 +844,7 @@ func (a *App) getGroupsAllowedForReferenceInChannel(channel *model.Channel, team groups, err := a.Srv().Store.Group().GetGroups(0, 0, opts) if err != nil { - return nil, model.NewAppError("getGroupsAllowedForReferenceInChannel", "app.select_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "unable to get groups") } for _, group := range groups { if group.Name != nil { diff --git a/app/notification_test.go b/app/notification_test.go index 10a0893891..e4e3ec6207 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -2465,15 +2465,14 @@ func TestGetGroupsAllowedForReferenceInChannel(t *testing.T) { defer th.TearDown() var err *model.AppError - var groupsMap map[string]*model.Group team := th.BasicTeam channel := th.BasicChannel group1 := th.CreateGroup() t.Run("should return empty map when no groups with allow reference", func(t *testing.T) { - groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) - require.Nil(t, err) + groupsMap, nErr := th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, nErr) require.Len(t, groupsMap, 0) }) @@ -2483,8 +2482,8 @@ func TestGetGroupsAllowedForReferenceInChannel(t *testing.T) { group2 := th.CreateGroup() t.Run("should only return groups with allow reference", func(t *testing.T) { - groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) - require.Nil(t, err) + groupsMap, nErr := th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, nErr) require.Len(t, groupsMap, 1) require.Nil(t, groupsMap[*group2.Name]) require.Equal(t, groupsMap[*group1.Name], group1) @@ -2507,8 +2506,8 @@ func TestGetGroupsAllowedForReferenceInChannel(t *testing.T) { require.Nil(t, err) t.Run("should return only groups synced to channel if channel is group constrained", func(t *testing.T) { - groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(constrainedChannel, team) - require.Nil(t, err) + groupsMap, nErr := th.App.getGroupsAllowedForReferenceInChannel(constrainedChannel, team) + require.Nil(t, nErr) require.Len(t, groupsMap, 1) require.Nil(t, groupsMap[*group2.Name]) require.Equal(t, groupsMap[*group1.Name], group1) @@ -2532,8 +2531,8 @@ func TestGetGroupsAllowedForReferenceInChannel(t *testing.T) { require.Nil(t, err) t.Run("should return union of groups synced to team and any channels if team is group constrained", func(t *testing.T) { - groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) - require.Nil(t, err) + groupsMap, nErr := th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, nErr) require.Len(t, groupsMap, 2) require.Nil(t, groupsMap[*group3.Name]) require.Equal(t, groupsMap[*group2.Name], group2) @@ -2541,8 +2540,8 @@ func TestGetGroupsAllowedForReferenceInChannel(t *testing.T) { }) t.Run("should return only subset of groups synced to channel for group constrained channel when team is also group constrained", func(t *testing.T) { - groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(constrainedChannel, team) - require.Nil(t, err) + groupsMap, nErr := th.App.getGroupsAllowedForReferenceInChannel(constrainedChannel, team) + require.Nil(t, nErr) require.Len(t, groupsMap, 1) require.Nil(t, groupsMap[*group3.Name]) require.Nil(t, groupsMap[*group2.Name]) @@ -2554,8 +2553,8 @@ func TestGetGroupsAllowedForReferenceInChannel(t *testing.T) { require.Nil(t, err) t.Run("should return all groups when team and channel are not group constrained", func(t *testing.T) { - groupsMap, err = th.App.getGroupsAllowedForReferenceInChannel(channel, team) - require.Nil(t, err) + groupsMap, nErr := th.App.getGroupsAllowedForReferenceInChannel(channel, team) + require.Nil(t, nErr) require.Len(t, groupsMap, 3) require.Equal(t, groupsMap[*group1.Name], group1) require.Equal(t, groupsMap[*group2.Name], group2) diff --git a/store/store.go b/store/store.go index a615093d7b..625c165b37 100644 --- a/store/store.go +++ b/store/store.go @@ -14,7 +14,6 @@ import ( type StoreResult struct { Data interface{} - Err *model.AppError // NErr a temporary field used by the new code for the AppError migration. This will later become Err when the entire store is migrated. NErr error