diff --git a/pkg/api/user.go b/pkg/api/user.go index ab9a951c657..bb471997438 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -114,7 +114,7 @@ func (hs *HTTPServer) GetUserByLoginOrEmail(c *models.ReqContext) response.Respo // 403: forbiddenError // 500: internalServerError func (hs *HTTPServer) UpdateSignedInUser(c *models.ReqContext) response.Response { - cmd := models.UpdateUserCommand{} + cmd := user.UpdateUserCommand{} if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } @@ -126,7 +126,7 @@ func (hs *HTTPServer) UpdateSignedInUser(c *models.ReqContext) response.Response return response.Error(400, "Not allowed to change username when auth proxy is using username property", nil) } } - cmd.UserId = c.UserId + cmd.UserID = c.UserId return hs.handleUpdateUser(c.Req.Context(), cmd) } @@ -143,12 +143,12 @@ func (hs *HTTPServer) UpdateSignedInUser(c *models.ReqContext) response.Response // 404: notFoundError // 500: internalServerError func (hs *HTTPServer) UpdateUser(c *models.ReqContext) response.Response { - cmd := models.UpdateUserCommand{} + cmd := user.UpdateUserCommand{} var err error if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } - cmd.UserId, err = strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64) + cmd.UserID, err = strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64) if err != nil { return response.Error(http.StatusBadRequest, "id is invalid", err) } @@ -179,7 +179,7 @@ func (hs *HTTPServer) UpdateUserActiveOrg(c *models.ReqContext) response.Respons return response.Success("Active organization changed") } -func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd models.UpdateUserCommand) response.Response { +func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd user.UpdateUserCommand) response.Response { if len(cmd.Login) == 0 { cmd.Login = cmd.Email if len(cmd.Login) == 0 { @@ -187,7 +187,7 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd models.UpdateUse } } - if err := hs.SQLStore.UpdateUser(ctx, &cmd); err != nil { + if err := hs.userService.Update(ctx, &cmd); err != nil { if errors.Is(err, user.ErrCaseInsensitive) { return response.Error(http.StatusConflict, "Update would result in user login conflict", err) } diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index 76bf618964b..ba9bda35d66 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -209,28 +209,28 @@ func (ls *Implementation) createUser(extUser *models.ExternalUserInfo) (*user.Us return ls.CreateUser(cmd) } -func (ls *Implementation) updateUser(ctx context.Context, user *user.User, extUser *models.ExternalUserInfo) error { +func (ls *Implementation) updateUser(ctx context.Context, usr *user.User, extUser *models.ExternalUserInfo) error { // sync user info - updateCmd := &models.UpdateUserCommand{ - UserId: user.ID, + updateCmd := &user.UpdateUserCommand{ + UserID: usr.ID, } needsUpdate := false - if extUser.Login != "" && extUser.Login != user.Login { + if extUser.Login != "" && extUser.Login != usr.Login { updateCmd.Login = extUser.Login - user.Login = extUser.Login + usr.Login = extUser.Login needsUpdate = true } - if extUser.Email != "" && extUser.Email != user.Email { + if extUser.Email != "" && extUser.Email != usr.Email { updateCmd.Email = extUser.Email - user.Email = extUser.Email + usr.Email = extUser.Email needsUpdate = true } - if extUser.Name != "" && extUser.Name != user.Name { + if extUser.Name != "" && extUser.Name != usr.Name { updateCmd.Name = extUser.Name - user.Name = extUser.Name + usr.Name = extUser.Name needsUpdate = true } @@ -238,8 +238,8 @@ func (ls *Implementation) updateUser(ctx context.Context, user *user.User, extUs return nil } - logger.Debug("Syncing user info", "id", user.ID, "update", updateCmd) - return ls.SQLStore.UpdateUser(ctx, updateCmd) + logger.Debug("Syncing user info", "id", usr.ID, "update", updateCmd) + return ls.userService.Update(ctx, updateCmd) } func (ls *Implementation) updateUserAuth(ctx context.Context, user *user.User, extUser *models.ExternalUserInfo) error { diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index cc4a0a8b6d1..f83aceee998 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -137,10 +137,6 @@ func (m *SQLStoreMock) CreateUser(ctx context.Context, cmd user.CreateUserComman return nil, m.ExpectedError } -func (m *SQLStoreMock) UpdateUser(ctx context.Context, cmd *models.UpdateUserCommand) error { - return m.ExpectedError -} - func (m *SQLStoreMock) ChangeUserPassword(ctx context.Context, cmd *models.ChangeUserPasswordCommand) error { return m.ExpectedError } diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index e2a84b1530f..5119b28ded8 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -31,7 +31,6 @@ type Store interface { GetUserLoginAttemptCount(ctx context.Context, query *models.GetUserLoginAttemptCountQuery) error DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error CreateUser(ctx context.Context, cmd user.CreateUserCommand) (*user.User, error) - UpdateUser(ctx context.Context, cmd *models.UpdateUserCommand) error ChangeUserPassword(ctx context.Context, cmd *models.ChangeUserPasswordCommand) error UpdateUserLastSeenAt(ctx context.Context, cmd *models.UpdateUserLastSeenAtCommand) error SetUsingOrg(ctx context.Context, cmd *models.SetUsingOrgCommand) error diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index 0983886355a..2f3d05a7f16 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -66,6 +66,15 @@ type GetUserByEmailQuery struct { Email string } +type UpdateUserCommand struct { + Name string `json:"name"` + Email string `json:"email"` + Login string `json:"login"` + Theme string `json:"theme"` + + UserID int64 `json:"-"` +} + func (u *User) NameOrFallback() string { if u.Name != "" { return u.Name diff --git a/pkg/services/user/user.go b/pkg/services/user/user.go index 6863fc3a6bd..f6d3e9e6268 100644 --- a/pkg/services/user/user.go +++ b/pkg/services/user/user.go @@ -10,4 +10,5 @@ type Service interface { GetByID(context.Context, *GetUserByIDQuery) (*User, error) GetByLogin(context.Context, *GetUserByLoginQuery) (*User, error) GetByEmail(context.Context, *GetUserByEmailQuery) (*User, error) + Update(context.Context, *UpdateUserCommand) error } diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index aab551a0cb0..599401d13cb 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -267,3 +267,15 @@ func (s *Service) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuer } return q.Result, nil } + +// TODO: remove wrapper around sqlstore +func (s *Service) Update(ctx context.Context, cmd *user.UpdateUserCommand) error { + q := &models.UpdateUserCommand{ + Name: cmd.Name, + Email: cmd.Email, + Login: cmd.Login, + Theme: cmd.Theme, + UserId: cmd.UserID, + } + return s.sqlStore.UpdateUser(ctx, q) +} diff --git a/pkg/services/user/usertest/fake.go b/pkg/services/user/usertest/fake.go index d546b940982..205c7c605f3 100644 --- a/pkg/services/user/usertest/fake.go +++ b/pkg/services/user/usertest/fake.go @@ -34,3 +34,7 @@ func (f *FakeUserService) GetByLogin(ctx context.Context, query *user.GetUserByL func (f *FakeUserService) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuery) (*user.User, error) { return f.ExpectedUser, f.ExpectedError } + +func (f *FakeUserService) Update(ctx context.Context, cmd *user.UpdateUserCommand) error { + return f.ExpectedError +}