From 23f3a55cb62953580f1bf692496f462d58f90547 Mon Sep 17 00:00:00 2001 From: Mylon Suren <23694620+mylonsuren@users.noreply.github.com> Date: Mon, 7 Mar 2022 19:16:45 -0500 Subject: [PATCH] [MM-41815] Fix custom status not handling update user errors (#19588) Co-authored-by: Mattermod --- app/status.go | 4 +-- app/status_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/app/status.go b/app/status.go index 2b21b4750f..c3d34d8048 100644 --- a/app/status.go +++ b/app/status.go @@ -418,7 +418,7 @@ func (a *App) SetCustomStatus(userID string, cs *model.CustomStatus) *model.AppE user.SetCustomStatus(cs) _, updateErr := a.UpdateUser(user, true) if updateErr != nil { - return err + return updateErr } if err := a.addRecentCustomStatus(userID, cs); err != nil { @@ -437,7 +437,7 @@ func (a *App) RemoveCustomStatus(userID string) *model.AppError { user.ClearCustomStatus() _, updateErr := a.UpdateUser(user, true) if updateErr != nil { - return err + return updateErr } return nil diff --git a/app/status_test.go b/app/status_test.go index c7f3a45a26..74d82f5153 100644 --- a/app/status_test.go +++ b/app/status_test.go @@ -6,9 +6,13 @@ package app import ( "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/v6/app/users" "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/store" + "github.com/mattermost/mattermost-server/v6/store/storetest/mocks" ) func TestSaveStatus(t *testing.T) { @@ -64,3 +68,62 @@ func TestCustomStatus(t *testing.T) { require.Nil(t, err, "failed to get custom status after clear %v", err) require.Equal(t, csClear, csSaved) } + +func TestCustomStatusErrors(t *testing.T) { + + fakeUserID := "foobar" + mockErr := store.NewErrNotFound("User", fakeUserID) + mockUser := &model.User{Id: fakeUserID} + + tests := map[string]struct { + customStatus string + successFn string + failFn string + expectedErr string + }{ + "set custom status fails on get user": {customStatus: "set", successFn: "Update", failFn: "Get", expectedErr: MissingAccountError}, + "set custom status fails on update user": {customStatus: "set", successFn: "Get", failFn: "Update", expectedErr: "app.user.update.finding.app_error"}, + "remove custom status fails on get user": {customStatus: "remove", successFn: "Update", failFn: "Get", expectedErr: MissingAccountError}, + "remove custom status fails on update user": {customStatus: "remove", successFn: "Get", failFn: "Update", expectedErr: "app.user.update.finding.app_error"}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() + + mockUserStore := mocks.UserStore{} + + mockUserStore.On(tc.successFn, mock.Anything, mock.Anything).Return(mockUser, nil) + mockUserStore.On(tc.failFn, mock.Anything, mock.Anything).Return(nil, mockErr) + + var err error + mockSessionStore := mocks.SessionStore{} + mockOAuthStore := mocks.OAuthStore{} + th.App.ch.srv.userService, err = users.New(users.ServiceConfig{ + UserStore: &mockUserStore, + SessionStore: &mockSessionStore, + OAuthStore: &mockOAuthStore, + ConfigFn: th.App.ch.srv.Config, + LicenseFn: th.App.ch.srv.License, + }) + require.NoError(t, err) + + cs := &model.CustomStatus{ + Emoji: ":smile:", + Text: "honk!", + } + + var appErr *model.AppError + switch tc.customStatus { + case "set": + appErr = th.App.SetCustomStatus(fakeUserID, cs) + case "remove": + appErr = th.App.RemoveCustomStatus(fakeUserID) + } + + require.NotNil(t, appErr) + require.Equal(t, tc.expectedErr, appErr.Id) + }) + } +}