Update user store to return the updated entities after inserting / patching them (#3843)

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)

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

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

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
 	}
 

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)

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
 	}

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},

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

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"`
 }

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,

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.

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

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}).

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)

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 {

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