diff --git a/server/api/users.go b/server/api/users.go index 61568a874..d4dcb6895 100644 --- a/server/api/users.go +++ b/server/api/users.go @@ -115,7 +115,6 @@ func (a *API) handleGetMe(w http.ResponseWriter, r *http.Request) { Email: model.SingleUser, CreateAt: ws.UpdateAt, UpdateAt: now, - Props: map[string]interface{}{}, } } else { user, err = a.app.GetUser(userID) @@ -280,7 +279,7 @@ func (a *API) handleUpdateUserConfig(w http.ResponseWriter, r *http.Request) { return } - var patch *model.UserPropPatch + var patch *model.UserPreferencesPatch err = json.Unmarshal(requestBody, &patch) if err != nil { a.errorResponse(w, r, err) diff --git a/server/app/auth.go b/server/app/auth.go index f36a449b6..ba1a77335 100644 --- a/server/app/auth.go +++ b/server/app/auth.go @@ -178,7 +178,7 @@ func (a *App) RegisterUser(username, email, password string) error { return errors.Wrap(err, "Invalid password") } - err = a.store.CreateUser(&model.User{ + _, err = a.store.CreateUser(&model.User{ ID: utils.NewID(utils.IDTypeUser), Username: username, Email: email, @@ -186,7 +186,6 @@ func (a *App) RegisterUser(username, email, password string) error { MfaSecret: "", AuthService: a.config.AuthMode, AuthData: "", - Props: map[string]interface{}{}, }) if err != nil { return errors.Wrap(err, "Unable to create the new user") diff --git a/server/app/auth_test.go b/server/app/auth_test.go index 14cef318f..8478c3126 100644 --- a/server/app/auth_test.go +++ b/server/app/auth_test.go @@ -109,7 +109,7 @@ func TestRegisterUser(t *testing.T) { th.Store.EXPECT().GetUserByUsername("newUsername").Return(mockUser, errors.New("user not found")) th.Store.EXPECT().GetUserByEmail("existingEmail").Return(mockUser, nil) th.Store.EXPECT().GetUserByEmail("newEmail").Return(nil, model.NewErrNotFound("user")) - th.Store.EXPECT().CreateUser(gomock.Any()).Return(nil) + th.Store.EXPECT().CreateUser(gomock.Any()).Return(nil, nil) for _, test := range testcases { t.Run(test.title, func(t *testing.T) { diff --git a/server/app/onboarding.go b/server/app/onboarding.go index 28683b47b..028c6dda1 100644 --- a/server/app/onboarding.go +++ b/server/app/onboarding.go @@ -30,14 +30,14 @@ func (a *App) PrepareOnboardingTour(userID string, teamID string) (string, strin } // set user's tour state to initial state - userPropPatch := model.UserPropPatch{ + userPreferencesPatch := model.UserPreferencesPatch{ UpdatedFields: map[string]string{ KeyOnboardingTourStarted: "1", KeyOnboardingTourStep: ValueOnboardingFirstStep, KeyOnboardingTourCategory: ValueTourCategoryOnboarding, }, } - if err := a.store.PatchUserProps(userID, userPropPatch); err != nil { + if _, err := a.store.PatchUserPreferences(userID, userPreferencesPatch); err != nil { return "", "", err } diff --git a/server/app/onboarding_test.go b/server/app/onboarding_test.go index 17cd665a5..26787413a 100644 --- a/server/app/onboarding_test.go +++ b/server/app/onboarding_test.go @@ -42,7 +42,7 @@ func TestPrepareOnboardingTour(t *testing.T) { newType := model.BoardTypePrivate th.Store.EXPECT().PatchBoard("board_id_1", &model.BoardPatch{Type: &newType}, "user_id_1").Return(&privateWelcomeBoard, nil) - userPropPatch := model.UserPropPatch{ + userPreferencesPatch := model.UserPreferencesPatch{ UpdatedFields: map[string]string{ KeyOnboardingTourStarted: "1", KeyOnboardingTourStep: ValueOnboardingFirstStep, @@ -50,7 +50,7 @@ func TestPrepareOnboardingTour(t *testing.T) { }, } - th.Store.EXPECT().PatchUserProps(userID, userPropPatch).Return(nil) + th.Store.EXPECT().PatchUserPreferences(userID, userPreferencesPatch).Return(nil, nil) th.Store.EXPECT().GetUserCategoryBoards(userID, "0").Return([]model.CategoryBoards{}, nil) teamID, boardID, err := th.App.PrepareOnboardingTour(userID, teamID) diff --git a/server/app/user.go b/server/app/user.go index c61dbbdef..62c710138 100644 --- a/server/app/user.go +++ b/server/app/user.go @@ -13,12 +13,8 @@ func (a *App) SearchTeamUsers(teamID string, searchQuery string, asGuestID strin return a.store.SearchUsersByTeam(teamID, searchQuery, asGuestID, excludeBots) } -func (a *App) UpdateUserConfig(userID string, patch model.UserPropPatch) ([]mmModel.Preference, error) { - if err := a.store.PatchUserProps(userID, patch); err != nil { - return nil, err - } - - updatedPreferences, err := a.store.GetUserPreferences(userID) +func (a *App) UpdateUserConfig(userID string, patch model.UserPreferencesPatch) ([]mmModel.Preference, error) { + updatedPreferences, err := a.store.PatchUserPreferences(userID, patch) if err != nil { return nil, err } diff --git a/server/integrationtests/permissions_test.go b/server/integrationtests/permissions_test.go index 70dfdc9ae..a5688ad6c 100644 --- a/server/integrationtests/permissions_test.go +++ b/server/integrationtests/permissions_test.go @@ -2561,7 +2561,7 @@ func TestPermissionsUserChangePassword(t *testing.T) { } func TestPermissionsUpdateUserConfig(t *testing.T) { - patch := toJSON(t, model.UserPropPatch{UpdatedFields: map[string]string{"test": "test"}}) + patch := toJSON(t, model.UserPreferencesPatch{UpdatedFields: map[string]string{"test": "test"}}) ttCases := []TestCase{ {"/users/{USER_TEAM_MEMBER_ID}/config", methodPut, patch, userAnon, http.StatusUnauthorized, 0}, diff --git a/server/integrationtests/pluginteststore.go b/server/integrationtests/pluginteststore.go index 232e14fd6..6c5109fe6 100644 --- a/server/integrationtests/pluginteststore.go +++ b/server/integrationtests/pluginteststore.go @@ -27,7 +27,6 @@ func NewPluginTestStore(innerStore store.Store) *PluginTestStore { users: map[string]*model.User{ "no-team-member": { ID: "no-team-member", - Props: map[string]interface{}{}, Username: "no-team-member", Email: "no-team-member@sample.com", CreateAt: model.GetMillis(), @@ -35,7 +34,6 @@ func NewPluginTestStore(innerStore store.Store) *PluginTestStore { }, "team-member": { ID: "team-member", - Props: map[string]interface{}{}, Username: "team-member", Email: "team-member@sample.com", CreateAt: model.GetMillis(), @@ -43,7 +41,6 @@ func NewPluginTestStore(innerStore store.Store) *PluginTestStore { }, "viewer": { ID: "viewer", - Props: map[string]interface{}{}, Username: "viewer", Email: "viewer@sample.com", CreateAt: model.GetMillis(), @@ -51,7 +48,6 @@ func NewPluginTestStore(innerStore store.Store) *PluginTestStore { }, "commenter": { ID: "commenter", - Props: map[string]interface{}{}, Username: "commenter", Email: "commenter@sample.com", CreateAt: model.GetMillis(), @@ -59,7 +55,6 @@ func NewPluginTestStore(innerStore store.Store) *PluginTestStore { }, "editor": { ID: "editor", - Props: map[string]interface{}{}, Username: "editor", Email: "editor@sample.com", CreateAt: model.GetMillis(), @@ -67,7 +62,6 @@ func NewPluginTestStore(innerStore store.Store) *PluginTestStore { }, "admin": { ID: "admin", - Props: map[string]interface{}{}, Username: "admin", Email: "admin@sample.com", CreateAt: model.GetMillis(), @@ -75,7 +69,6 @@ func NewPluginTestStore(innerStore store.Store) *PluginTestStore { }, "guest": { ID: "guest", - Props: map[string]interface{}{}, Username: "guest", Email: "guest@sample.com", CreateAt: model.GetMillis(), @@ -150,27 +143,6 @@ func (s *PluginTestStore) GetUserByUsername(username string) (*model.User, error return nil, errTestStore } -func (s *PluginTestStore) PatchUserProps(userID string, patch model.UserPropPatch) error { - user, err := s.GetUserByID(userID) - if err != nil { - return err - } - - props := user.Props - - for _, key := range patch.DeletedFields { - delete(props, key) - } - - for key, value := range patch.UpdatedFields { - props[key] = value - } - - user.Props = props - - return nil -} - func (s *PluginTestStore) GetUserPreferences(userID string) (mmModel.Preferences, error) { if userID == userTeamMember { return mmModel.Preferences{{ diff --git a/server/model/user.go b/server/model/user.go index 740d91445..7b48cb0a4 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -46,10 +46,6 @@ type User struct { // swagger:ignore AuthData string `json:"-"` - // User settings - // required: true - Props map[string]interface{} `json:"props"` - // Created time in miliseconds since the current epoch // required: true CreateAt int64 `json:"create_at,omitempty"` @@ -73,14 +69,14 @@ type User struct { Roles string `json:"roles"` } -// UserPropPatch is a user property patch +// UserPreferencesPatch is a user property patch // swagger:model -type UserPropPatch struct { - // The user prop updated fields +type UserPreferencesPatch struct { + // The user preference updated fields // required: false UpdatedFields map[string]string `json:"updatedFields"` - // The user prop removed fields + // The user preference removed fields // required: false DeletedFields []string `json:"deletedFields"` } diff --git a/server/services/store/mattermostauthlayer/mattermostauthlayer.go b/server/services/store/mattermostauthlayer/mattermostauthlayer.go index ce70b3693..e2c7d7545 100644 --- a/server/services/store/mattermostauthlayer/mattermostauthlayer.go +++ b/server/services/store/mattermostauthlayer/mattermostauthlayer.go @@ -115,12 +115,12 @@ func (s *MattermostAuthLayer) GetUserByUsername(username string) (*model.User, e return &user, nil } -func (s *MattermostAuthLayer) CreateUser(user *model.User) error { - return store.NewNotSupportedError("no user creation allowed from focalboard, create it using mattermost") +func (s *MattermostAuthLayer) CreateUser(user *model.User) (*model.User, error) { + return nil, store.NewNotSupportedError("no user creation allowed from focalboard, create it using mattermost") } -func (s *MattermostAuthLayer) UpdateUser(user *model.User) error { - return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost") +func (s *MattermostAuthLayer) UpdateUser(user *model.User) (*model.User, error) { + return nil, store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost") } func (s *MattermostAuthLayer) UpdateUserPassword(username, password string) error { @@ -131,7 +131,12 @@ func (s *MattermostAuthLayer) UpdateUserPasswordByID(userID, password string) er return store.NewNotSupportedError("no update allowed from focalboard, update it using mattermost") } -func (s *MattermostAuthLayer) PatchUserProps(userID string, patch model.UserPropPatch) error { +func (s *MattermostAuthLayer) PatchUserPreferences(userID string, patch model.UserPreferencesPatch) (mmModel.Preferences, error) { + preferences, err := s.GetUserPreferences(userID) + if err != nil { + return nil, err + } + if len(patch.UpdatedFields) > 0 { updatedPreferences := mmModel.Preferences{} for key, value := range patch.UpdatedFields { @@ -147,8 +152,27 @@ func (s *MattermostAuthLayer) PatchUserProps(userID string, patch model.UserProp if err := s.servicesAPI.UpdatePreferencesForUser(userID, updatedPreferences); err != nil { s.logger.Error("failed to update user preferences", mlog.String("user_id", userID), mlog.Err(err)) - return err + return nil, err } + + // we update the preferences list replacing or adding those + // that were updated + newPreferences := mmModel.Preferences{} + for _, existingPreference := range preferences { + hasBeenUpdated := false + for _, updatedPreference := range updatedPreferences { + if updatedPreference.Name == existingPreference.Name { + hasBeenUpdated = true + break + } + } + + if !hasBeenUpdated { + newPreferences = append(newPreferences, existingPreference) + } + } + newPreferences = append(newPreferences, updatedPreferences...) + preferences = newPreferences } if len(patch.DeletedFields) > 0 { @@ -165,11 +189,29 @@ func (s *MattermostAuthLayer) PatchUserProps(userID string, patch model.UserProp if err := s.servicesAPI.DeletePreferencesForUser(userID, deletedPreferences); err != nil { s.logger.Error("failed to delete user preferences", mlog.String("user_id", userID), mlog.Err(err)) - return err + return nil, err } + + // we update the preferences removing those that have been + // deleted + newPreferences := mmModel.Preferences{} + for _, existingPreference := range preferences { + hasBeenDeleted := false + for _, deletedPreference := range deletedPreferences { + if deletedPreference.Name == existingPreference.Name { + hasBeenDeleted = true + break + } + } + + if !hasBeenDeleted { + newPreferences = append(newPreferences, existingPreference) + } + } + preferences = newPreferences } - return nil + return preferences, nil } func (s *MattermostAuthLayer) GetUserPreferences(userID string) (mmModel.Preferences, error) { @@ -291,7 +333,7 @@ func (s *MattermostAuthLayer) getQueryBuilder() sq.StatementBuilderType { func (s *MattermostAuthLayer) GetUsersByTeam(teamID string, asGuestID string) ([]*model.User, error) { query := s.getQueryBuilder(). - Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.props", "u.CreateAt as create_at", "u.UpdateAt as update_at", + Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.CreateAt as create_at", "u.UpdateAt as update_at", "u.DeleteAt as delete_at", "b.UserId IS NOT NULL AS is_bot, u.roles = 'system_guest' as is_guest"). From("Users as u"). LeftJoin("Bots b ON ( b.UserID = u.id )"). @@ -332,7 +374,7 @@ func (s *MattermostAuthLayer) GetUsersByTeam(teamID string, asGuestID string) ([ func (s *MattermostAuthLayer) GetUsersList(userIDs []string) ([]*model.User, error) { query := s.getQueryBuilder(). - Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.props", "u.CreateAt as create_at", "u.UpdateAt as update_at", + Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.CreateAt as create_at", "u.UpdateAt as update_at", "u.DeleteAt as delete_at", "b.UserId IS NOT NULL AS is_bot, u.roles = 'system_guest' as is_guest"). From("Users as u"). LeftJoin("Bots b ON ( b.UserId = u.id )"). @@ -358,7 +400,7 @@ func (s *MattermostAuthLayer) GetUsersList(userIDs []string) ([]*model.User, err func (s *MattermostAuthLayer) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) { query := s.getQueryBuilder(). - Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.props", "u.CreateAt as create_at", "u.UpdateAt as update_at", + Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.CreateAt as create_at", "u.UpdateAt as update_at", "u.DeleteAt as delete_at", "b.UserId IS NOT NULL AS is_bot, u.roles = 'system_guest' as is_guest"). From("Users as u"). LeftJoin("Bots b ON ( b.UserId = u.id )"). @@ -414,7 +456,6 @@ func (s *MattermostAuthLayer) usersFromRows(rows *sql.Rows) ([]*model.User, erro for rows.Next() { var user model.User - var propsBytes []byte err := rows.Scan( &user.ID, @@ -423,7 +464,6 @@ func (s *MattermostAuthLayer) usersFromRows(rows *sql.Rows) ([]*model.User, erro &user.Nickname, &user.FirstName, &user.LastName, - &propsBytes, &user.CreateAt, &user.UpdateAt, &user.DeleteAt, @@ -434,11 +474,6 @@ func (s *MattermostAuthLayer) usersFromRows(rows *sql.Rows) ([]*model.User, erro return nil, err } - err = json.Unmarshal(propsBytes, &user.Props) - if err != nil { - return nil, err - } - users = append(users, &user) } @@ -464,10 +499,6 @@ func (s *MattermostAuthLayer) CreatePrivateWorkspace(userID string) (string, err } func mmUserToFbUser(mmUser *mmModel.User) model.User { - props := map[string]interface{}{} - for key, value := range mmUser.Props { - props[key] = value - } authData := "" if mmUser.AuthData != nil { authData = *mmUser.AuthData @@ -483,7 +514,6 @@ func mmUserToFbUser(mmUser *mmModel.User) model.User { MfaSecret: mmUser.MfaSecret, AuthService: mmUser.AuthService, AuthData: authData, - Props: props, CreateAt: mmUser.CreateAt, UpdateAt: mmUser.UpdateAt, DeleteAt: mmUser.DeleteAt, diff --git a/server/services/store/mockstore/mockstore.go b/server/services/store/mockstore/mockstore.go index 4bc1c5dbc..77b9ea1df 100644 --- a/server/services/store/mockstore/mockstore.go +++ b/server/services/store/mockstore/mockstore.go @@ -154,11 +154,12 @@ func (mr *MockStoreMockRecorder) CreateSubscription(arg0 interface{}) *gomock.Ca } // CreateUser mocks base method. -func (m *MockStore) CreateUser(arg0 *model.User) error { +func (m *MockStore) CreateUser(arg0 *model.User) (*model.User, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateUser", arg0) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].(*model.User) + ret1, _ := ret[1].(error) + return ret0, ret1 } // CreateUser indicates an expected call of CreateUser. @@ -1282,18 +1283,19 @@ func (mr *MockStoreMockRecorder) PatchBoardsAndBlocks(arg0, arg1 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PatchBoardsAndBlocks", reflect.TypeOf((*MockStore)(nil).PatchBoardsAndBlocks), arg0, arg1) } -// PatchUserProps mocks base method. -func (m *MockStore) PatchUserProps(arg0 string, arg1 model.UserPropPatch) error { +// PatchUserPreferences mocks base method. +func (m *MockStore) PatchUserPreferences(arg0 string, arg1 model.UserPreferencesPatch) (model0.Preferences, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "PatchUserProps", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "PatchUserPreferences", arg0, arg1) + ret0, _ := ret[0].(model0.Preferences) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// PatchUserProps indicates an expected call of PatchUserProps. -func (mr *MockStoreMockRecorder) PatchUserProps(arg0, arg1 interface{}) *gomock.Call { +// PatchUserPreferences indicates an expected call of PatchUserPreferences. +func (mr *MockStoreMockRecorder) PatchUserPreferences(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PatchUserProps", reflect.TypeOf((*MockStore)(nil).PatchUserProps), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PatchUserPreferences", reflect.TypeOf((*MockStore)(nil).PatchUserPreferences), arg0, arg1) } // PostMessage mocks base method. @@ -1570,11 +1572,12 @@ func (mr *MockStoreMockRecorder) UpdateSubscribersNotifiedAt(arg0, arg1 interfac } // UpdateUser mocks base method. -func (m *MockStore) UpdateUser(arg0 *model.User) error { +func (m *MockStore) UpdateUser(arg0 *model.User) (*model.User, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UpdateUser", arg0) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].(*model.User) + ret1, _ := ret[1].(error) + return ret0, ret1 } // UpdateUser indicates an expected call of UpdateUser. diff --git a/server/services/store/sqlstore/public_methods.go b/server/services/store/sqlstore/public_methods.go index c663a0bfa..d81e1725e 100644 --- a/server/services/store/sqlstore/public_methods.go +++ b/server/services/store/sqlstore/public_methods.go @@ -119,7 +119,7 @@ func (s *SQLStore) CreateSubscription(sub *model.Subscription) (*model.Subscript } -func (s *SQLStore) CreateUser(user *model.User) error { +func (s *SQLStore) CreateUser(user *model.User) (*model.User, error) { return s.createUser(s.db, user) } @@ -722,8 +722,8 @@ func (s *SQLStore) PatchBoardsAndBlocks(pbab *model.PatchBoardsAndBlocks, userID } -func (s *SQLStore) PatchUserProps(userID string, patch model.UserPropPatch) error { - return s.patchUserProps(s.db, userID, patch) +func (s *SQLStore) PatchUserPreferences(userID string, patch model.UserPreferencesPatch) (mmModel.Preferences, error) { + return s.patchUserPreferences(s.db, userID, patch) } @@ -874,7 +874,7 @@ func (s *SQLStore) UpdateSubscribersNotifiedAt(blockID string, notifiedAt int64) } -func (s *SQLStore) UpdateUser(user *model.User) error { +func (s *SQLStore) UpdateUser(user *model.User) (*model.User, error) { return s.updateUser(s.db, user) } diff --git a/server/services/store/sqlstore/user.go b/server/services/store/sqlstore/user.go index eb995c63e..36c757f85 100644 --- a/server/services/store/sqlstore/user.go +++ b/server/services/store/sqlstore/user.go @@ -2,7 +2,6 @@ package sqlstore import ( "database/sql" - "encoding/json" "errors" "fmt" @@ -68,7 +67,6 @@ func (s *SQLStore) getUsersByCondition(db sq.BaseRunner, condition interface{}, "mfa_secret", "auth_service", "auth_data", - "props", "create_at", "update_at", "delete_at", @@ -125,52 +123,45 @@ func (s *SQLStore) getUserByUsername(db sq.BaseRunner, username string) (*model. return s.getUserByCondition(db, sq.Eq{"username": username}) } -func (s *SQLStore) createUser(db sq.BaseRunner, user *model.User) error { +func (s *SQLStore) createUser(db sq.BaseRunner, user *model.User) (*model.User, error) { now := utils.GetMillis() - - propsBytes, err := json.Marshal(user.Props) - if err != nil { - return err - } + user.CreateAt = now + user.UpdateAt = now + user.DeleteAt = 0 query := s.getQueryBuilder(db).Insert(s.tablePrefix+"users"). - Columns("id", "username", "email", "password", "mfa_secret", "auth_service", "auth_data", "props", "create_at", "update_at", "delete_at"). - Values(user.ID, user.Username, user.Email, user.Password, user.MfaSecret, user.AuthService, user.AuthData, propsBytes, now, now, 0) + Columns("id", "username", "email", "password", "mfa_secret", "auth_service", "auth_data", "create_at", "update_at", "delete_at"). + Values(user.ID, user.Username, user.Email, user.Password, user.MfaSecret, user.AuthService, user.AuthData, user.CreateAt, user.UpdateAt, user.DeleteAt) - _, err = query.Exec() - return err + _, err := query.Exec() + return user, err } -func (s *SQLStore) updateUser(db sq.BaseRunner, user *model.User) error { +func (s *SQLStore) updateUser(db sq.BaseRunner, user *model.User) (*model.User, error) { now := utils.GetMillis() - - propsBytes, err := json.Marshal(user.Props) - if err != nil { - return err - } + user.UpdateAt = now query := s.getQueryBuilder(db).Update(s.tablePrefix+"users"). Set("username", user.Username). Set("email", user.Email). - Set("props", propsBytes). - Set("update_at", now). + Set("update_at", user.UpdateAt). Where(sq.Eq{"id": user.ID}) result, err := query.Exec() if err != nil { - return err + return nil, err } rowCount, err := result.RowsAffected() if err != nil { - return err + return nil, err } if rowCount < 1 { - return UserNotFoundError{user.ID} + return nil, UserNotFoundError{user.ID} } - return nil + return user, nil } func (s *SQLStore) updateUserPassword(db sq.BaseRunner, username, password string) error { @@ -246,7 +237,6 @@ func (s *SQLStore) usersFromRows(rows *sql.Rows) ([]*model.User, error) { for rows.Next() { var user model.User - var propsBytes []byte err := rows.Scan( &user.ID, @@ -256,7 +246,6 @@ func (s *SQLStore) usersFromRows(rows *sql.Rows) ([]*model.User, error) { &user.MfaSecret, &user.AuthService, &user.AuthData, - &propsBytes, &user.CreateAt, &user.UpdateAt, &user.DeleteAt, @@ -265,18 +254,18 @@ func (s *SQLStore) usersFromRows(rows *sql.Rows) ([]*model.User, error) { return nil, err } - err = json.Unmarshal(propsBytes, &user.Props) - if err != nil { - return nil, err - } - users = append(users, &user) } return users, nil } -func (s *SQLStore) patchUserProps(db sq.BaseRunner, userID string, patch model.UserPropPatch) error { +func (s *SQLStore) patchUserPreferences(db sq.BaseRunner, userID string, patch model.UserPreferencesPatch) (mmModel.Preferences, error) { + preferences, err := s.getUserPreferences(db, userID) + if err != nil { + return nil, err + } + if len(patch.UpdatedFields) > 0 { for key, value := range patch.UpdatedFields { preference := mmModel.Preference{ @@ -286,9 +275,18 @@ func (s *SQLStore) patchUserProps(db sq.BaseRunner, userID string, patch model.U Value: value, } - if err := s.updateUserProps(db, preference); err != nil { - return err + if err := s.updateUserPreference(db, preference); err != nil { + return nil, err } + + newPreferences := mmModel.Preferences{} + for _, existingPreference := range preferences { + if preference.Name != existingPreference.Name { + newPreferences = append(newPreferences, existingPreference) + } + } + newPreferences = append(newPreferences, preference) + preferences = newPreferences } } @@ -300,16 +298,24 @@ func (s *SQLStore) patchUserProps(db sq.BaseRunner, userID string, patch model.U Name: key, } - if err := s.deleteUserProps(db, preference); err != nil { - return err + if err := s.deleteUserPreference(db, preference); err != nil { + return nil, err } + + newPreferences := mmModel.Preferences{} + for _, existingPreference := range preferences { + if preference.Name != existingPreference.Name { + newPreferences = append(newPreferences, existingPreference) + } + } + preferences = newPreferences } } - return nil + return preferences, nil } -func (s *SQLStore) updateUserProps(db sq.BaseRunner, preference mmModel.Preference) error { +func (s *SQLStore) updateUserPreference(db sq.BaseRunner, preference mmModel.Preference) error { query := s.getQueryBuilder(db). Insert(s.tablePrefix+"preferences"). Columns("UserId", "Category", "Name", "Value"). @@ -333,7 +339,7 @@ func (s *SQLStore) updateUserProps(db sq.BaseRunner, preference mmModel.Preferen return nil } -func (s *SQLStore) deleteUserProps(db sq.BaseRunner, preference mmModel.Preference) error { +func (s *SQLStore) deleteUserPreference(db sq.BaseRunner, preference mmModel.Preference) error { query := s.getQueryBuilder(db). Delete(s.tablePrefix + "preferences"). Where(sq.Eq{"UserId": preference.UserId}). diff --git a/server/services/store/store.go b/server/services/store/store.go index 7983091f5..e2841234e 100644 --- a/server/services/store/store.go +++ b/server/services/store/store.go @@ -59,13 +59,13 @@ type Store interface { GetUsersList(userIDs []string) ([]*model.User, error) GetUserByEmail(email string) (*model.User, error) GetUserByUsername(username string) (*model.User, error) - CreateUser(user *model.User) error - UpdateUser(user *model.User) error + CreateUser(user *model.User) (*model.User, error) + UpdateUser(user *model.User) (*model.User, error) UpdateUserPassword(username, password string) error UpdateUserPasswordByID(userID, password string) error GetUsersByTeam(teamID string, asGuestID string) ([]*model.User, error) SearchUsersByTeam(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) - PatchUserProps(userID string, patch model.UserPropPatch) error + PatchUserPreferences(userID string, patch model.UserPreferencesPatch) (mmModel.Preferences, error) GetUserPreferences(userID string) (mmModel.Preferences, error) GetActiveUserCount(updatedSecondsAgo int64) (int, error) diff --git a/server/services/store/storetests/users.go b/server/services/store/storetests/users.go index 7c5140209..0b1ebcbac 100644 --- a/server/services/store/storetests/users.go +++ b/server/services/store/storetests/users.go @@ -62,14 +62,17 @@ func testGetUsersByTeam(t *testing.T, store store.Store) { userID := utils.NewID(utils.IDTypeUser) - err = store.CreateUser(&model.User{ + user, err := store.CreateUser(&model.User{ ID: userID, Username: "darth.vader", }) require.NoError(t, err) + require.NotNil(t, user) + require.Equal(t, userID, user.ID) + require.Equal(t, "darth.vader", user.Username) defer func() { - _ = store.UpdateUser(&model.User{ + _, _ = store.UpdateUser(&model.User{ ID: userID, DeleteAt: utils.GetMillis(), }) @@ -90,8 +93,9 @@ func testCreateAndGetUser(t *testing.T, store store.Store) { } t.Run("CreateUser", func(t *testing.T) { - err := store.CreateUser(user) + newUser, err := store.CreateUser(user) require.NoError(t, err) + require.NotNil(t, newUser) }) t.Run("GetUserByID", func(t *testing.T) { @@ -147,8 +151,9 @@ func testGetUsersList(t *testing.T, store store.Store) { Username: fmt.Sprintf("%s-username", id), Email: fmt.Sprintf("%s@sample.com", id), } - err := store.CreateUser(user) + newUser, err := store.CreateUser(user) require.NoError(t, err) + require.NotNil(t, newUser) } testCases := []struct { @@ -200,22 +205,24 @@ func testCreateAndUpdateUser(t *testing.T, store store.Store) { user := &model.User{ ID: utils.NewID(utils.IDTypeUser), } - err := store.CreateUser(user) + newUser, err := store.CreateUser(user) require.NoError(t, err) + require.NotNil(t, newUser) t.Run("UpdateUser", func(t *testing.T) { user.Username = "damao" user.Email = "mock@email.com" - user.Props = map[string]interface{}{"a": "b"} - err := store.UpdateUser(user) + uUser, err := store.UpdateUser(user) require.NoError(t, err) + require.NotNil(t, uUser) + require.Equal(t, user.Username, uUser.Username) + require.Equal(t, user.Email, uUser.Email) got, err := store.GetUserByID(user.ID) require.NoError(t, err) require.Equal(t, user.ID, got.ID) require.Equal(t, user.Username, got.Username) require.Equal(t, user.Email, got.Email) - require.Equal(t, user.Props, got.Props) }) t.Run("UpdateUserPassword", func(t *testing.T) { @@ -244,10 +251,11 @@ func testCreateAndUpdateUser(t *testing.T, store store.Store) { func testCreateAndGetRegisteredUserCount(t *testing.T, store store.Store) { randomN := int(time.Now().Unix() % 10) for i := 0; i < randomN; i++ { - err := store.CreateUser(&model.User{ + user, err := store.CreateUser(&model.User{ ID: utils.NewID(utils.IDTypeUser), }) require.NoError(t, err) + require.NotNil(t, user) } got, err := store.GetRegisteredUserCount() @@ -259,15 +267,16 @@ func testPatchUserProps(t *testing.T, store store.Store) { user := &model.User{ ID: utils.NewID(utils.IDTypeUser), } - err := store.CreateUser(user) + newUser, err := store.CreateUser(user) require.NoError(t, err) + require.NotNil(t, newUser) key1 := "new_key_1" key2 := "new_key_2" key3 := "new_key_3" // Only update props - patch := model.UserPropPatch{ + patch := model.UserPreferencesPatch{ UpdatedFields: map[string]string{ key1: "new_value_1", key2: "new_value_2", @@ -275,9 +284,7 @@ func testPatchUserProps(t *testing.T, store store.Store) { }, } - err = store.PatchUserProps(user.ID, patch) - require.NoError(t, err) - userPreferences, err := store.GetUserPreferences(user.ID) + userPreferences, err := store.PatchUserPreferences(user.ID, patch) require.NoError(t, err) require.Equal(t, 3, len(userPreferences)) @@ -293,15 +300,13 @@ func testPatchUserProps(t *testing.T, store store.Store) { } // Delete a prop - patch = model.UserPropPatch{ + patch = model.UserPreferencesPatch{ DeletedFields: []string{ key1, }, } - err = store.PatchUserProps(user.ID, patch) - require.NoError(t, err) - userPreferences, err = store.GetUserPreferences(user.ID) + userPreferences, err = store.PatchUserPreferences(user.ID, patch) require.NoError(t, err) for _, preference := range userPreferences { @@ -316,7 +321,7 @@ func testPatchUserProps(t *testing.T, store store.Store) { } // update and delete together - patch = model.UserPropPatch{ + patch = model.UserPreferencesPatch{ UpdatedFields: map[string]string{ key3: "new_value_3_new_again", }, @@ -324,9 +329,7 @@ func testPatchUserProps(t *testing.T, store store.Store) { key2, }, } - err = store.PatchUserProps(user.ID, patch) - require.NoError(t, err) - userPreferences, err = store.GetUserPreferences(user.ID) + userPreferences, err = store.PatchUserPreferences(user.ID, patch) require.NoError(t, err) for _, preference := range userPreferences { diff --git a/server/services/store/storetests/util.go b/server/services/store/storetests/util.go index b7a01df60..40595d8d3 100644 --- a/server/services/store/storetests/util.go +++ b/server/services/store/storetests/util.go @@ -21,8 +21,9 @@ func createTestUsers(t *testing.T, store store.Store, num int) []*model.User { Username: fmt.Sprintf("mooncake.%d", i), Email: fmt.Sprintf("mooncake.%d@example.com", i), } - err := store.CreateUser(user) + newUser, err := store.CreateUser(user) require.NoError(t, err) + require.NotNil(t, newUser) users = append(users, user) }