Chore: Copy sqlstore methods to user store (#56280)

* Chore: Copy sqlstore methods to user store

* Fix xorm tag

* Add tests

* Remove unused methods from sqlstore

pkg/api/admin_users.go

@@ -159,7 +159,7 @@ func (hs *HTTPServer) AdminUpdateUserPermissions(c *models.ReqContext) response.
 		return response.Error(http.StatusBadRequest, "id is invalid", err)
 	}
 
-	err = hs.userService.UpdatePermissions(userID, form.IsGrafanaAdmin)
+	err = hs.userService.UpdatePermissions(c.Req.Context(), userID, form.IsGrafanaAdmin)
 	if err != nil {
 		if errors.Is(err, user.ErrLastGrafanaAdmin) {
 			return response.Error(400, user.ErrLastGrafanaAdmin.Error(), nil)

pkg/services/login/loginservice/loginservice.go

@@ -148,7 +148,7 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser
 
 	// Sync isGrafanaAdmin permission
 	if extUser.IsGrafanaAdmin != nil && *extUser.IsGrafanaAdmin != cmd.Result.IsAdmin {
-		if errPerms := ls.userService.UpdatePermissions(cmd.Result.ID, *extUser.IsGrafanaAdmin); errPerms != nil {
+		if errPerms := ls.userService.UpdatePermissions(ctx, cmd.Result.ID, *extUser.IsGrafanaAdmin); errPerms != nil {
 			return errPerms
 		}
 	}

pkg/services/sqlstore/store.go

@@ -27,8 +27,6 @@ type Store interface {
 	GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error
 	GetUserOrgList(ctx context.Context, query *models.GetUserOrgListQuery) error
 	GetSignedInUser(ctx context.Context, query *models.GetSignedInUserQuery) error
-	UpdateUserPermissions(userID int64, isAdmin bool) error
-	SetUserHelpFlag(ctx context.Context, cmd *models.SetUserHelpFlagCommand) error
 	WithDbSession(ctx context.Context, callback DBTransactionFunc) error
 	WithNewDbSession(ctx context.Context, callback DBTransactionFunc) error
 	GetOrgQuotaByTarget(ctx context.Context, query *models.GetOrgQuotaByTargetQuery) error

pkg/services/sqlstore/user.go

@@ -702,52 +702,3 @@ func UserDeletions() []string {
 	}
 	return deletes
 }
-
-// UpdateUserPermissions sets the user Server Admin flag
-func (ss *SQLStore) UpdateUserPermissions(userID int64, isAdmin bool) error {
-	return ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error {
-		var user user.User
-		if _, err := sess.ID(userID).Where(NotServiceAccountFilter(ss)).Get(&user); err != nil {
-			return err
-		}
-
-		user.IsAdmin = isAdmin
-		sess.UseBool("is_admin")
-		_, err := sess.ID(user.ID).Update(&user)
-		if err != nil {
-			return err
-		}
-		// validate that after update there is at least one server admin
-		if err := validateOneAdminLeft(sess); err != nil {
-			return err
-		}
-		return nil
-	})
-}
-
-func (ss *SQLStore) SetUserHelpFlag(ctx context.Context, cmd *models.SetUserHelpFlagCommand) error {
-	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
-		user := user.User{
-			ID:         cmd.UserId,
-			HelpFlags1: cmd.HelpFlags1,
-			Updated:    TimeNow(),
-		}
-
-		_, err := sess.ID(cmd.UserId).Cols("help_flags1").Update(&user)
-		return err
-	})
-}
-
-// validateOneAdminLeft validate that there is an admin user left
-func validateOneAdminLeft(sess *DBSession) error {
-	count, err := sess.Where("is_admin=?", true).Count(&user.User{})
-	if err != nil {
-		return err
-	}
-
-	if count == 0 {
-		return user.ErrLastGrafanaAdmin
-	}
-
-	return nil
-}

pkg/services/sqlstore/user_test.go

@@ -419,61 +419,6 @@ func TestIntegrationUserDataAccess(t *testing.T) {
 			}
 		})
 	})
-
-	t.Run("Testing DB - grafana admin users", func(t *testing.T) {
-		ss = InitTestDB(t)
-
-		createUserCmd := user.CreateUserCommand{
-			Email:   fmt.Sprint("admin", "@test.com"),
-			Name:    "admin",
-			Login:   "admin",
-			IsAdmin: true,
-		}
-		usr, err := ss.CreateUser(context.Background(), createUserCmd)
-		require.Nil(t, err)
-
-		// Cannot make themselves a non-admin
-		updatePermsError := ss.UpdateUserPermissions(usr.ID, false)
-
-		require.Equal(t, updatePermsError, user.ErrLastGrafanaAdmin)
-
-		query := models.GetUserByIdQuery{Id: usr.ID}
-		getUserError := ss.GetUserById(context.Background(), &query)
-		require.Nil(t, getUserError)
-
-		require.True(t, query.Result.IsAdmin)
-
-		// One user
-		const email = "user@test.com"
-		const username = "user"
-		createUserCmd = user.CreateUserCommand{
-			Email: email,
-			Name:  "user",
-			Login: username,
-		}
-		_, err = ss.CreateUser(context.Background(), createUserCmd)
-		require.Nil(t, err)
-
-		// When trying to create a new user with the same email, an error is returned
-		createUserCmd = user.CreateUserCommand{
-			Email:        email,
-			Name:         "user2",
-			Login:        "user2",
-			SkipOrgSetup: true,
-		}
-		_, err = ss.CreateUser(context.Background(), createUserCmd)
-		require.Equal(t, err, user.ErrUserAlreadyExists)
-
-		// When trying to create a new user with the same login, an error is returned
-		createUserCmd = user.CreateUserCommand{
-			Email:        "user2@test.com",
-			Name:         "user2",
-			Login:        username,
-			SkipOrgSetup: true,
-		}
-		_, err = ss.CreateUser(context.Background(), createUserCmd)
-		require.Equal(t, err, user.ErrUserAlreadyExists)
-	})
 }
 
 func (ss *SQLStore) GetOrgUsersForTest(ctx context.Context, query *models.GetOrgUsersQuery) error {

pkg/services/user/model.go

@@ -182,7 +182,7 @@ type BatchDisableUsersCommand struct {
 
 type SetUserHelpFlagCommand struct {
 	HelpFlags1 HelpFlags1
-	UserID     int64
+	UserID     int64 `xorm:"user_id"`
 }
 
 type GetSignedInUserQuery struct {

pkg/services/user/user.go

@@ -19,7 +19,7 @@ type Service interface {
 	Search(context.Context, *SearchUsersQuery) (*SearchUserQueryResult, error)
 	Disable(context.Context, *DisableUserCommand) error
 	BatchDisableUsers(context.Context, *BatchDisableUsersCommand) error
-	UpdatePermissions(int64, bool) error
+	UpdatePermissions(context.Context, int64, bool) error
 	SetUserHelpFlag(context.Context, *SetUserHelpFlagCommand) error
-	GetProfile(context.Context, *GetUserProfileQuery) (UserProfileDTO, error)
+	GetProfile(context.Context, *GetUserProfileQuery) (*UserProfileDTO, error)
 }

pkg/services/user/userimpl/store.go

@@ -30,6 +30,9 @@ type store interface {
 	UpdateLastSeenAt(context.Context, *user.UpdateUserLastSeenAtCommand) error
 	GetSignedInUser(context.Context, *user.GetSignedInUserQuery) (*user.SignedInUser, error)
 	UpdateUser(context.Context, *user.User) error
+	GetProfile(context.Context, *user.GetUserProfileQuery) (*user.UserProfileDTO, error)
+	SetHelpFlag(context.Context, *user.SetUserHelpFlagCommand) error
+	UpdatePermissions(context.Context, int64, bool) error
 }
 
 type sqlStore struct {
@@ -387,3 +390,82 @@ func (ss *sqlStore) UpdateUser(ctx context.Context, user *user.User) error {
 		return err
 	})
 }
+
+func (ss *sqlStore) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) {
+	var usr user.User
+	var userProfile user.UserProfileDTO
+	err := ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		has, err := sess.ID(query.UserID).Where(ss.notServiceAccountFilter()).Get(&usr)
+
+		if err != nil {
+			return err
+		} else if !has {
+			return user.ErrUserNotFound
+		}
+
+		userProfile = user.UserProfileDTO{
+			ID:             usr.ID,
+			Name:           usr.Name,
+			Email:          usr.Email,
+			Login:          usr.Login,
+			Theme:          usr.Theme,
+			IsGrafanaAdmin: usr.IsAdmin,
+			IsDisabled:     usr.IsDisabled,
+			OrgID:          usr.OrgID,
+			UpdatedAt:      usr.Updated,
+			CreatedAt:      usr.Created,
+		}
+
+		return err
+	})
+	return &userProfile, err
+}
+
+func (ss *sqlStore) SetHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCommand) error {
+	return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		user := user.User{
+			ID:         cmd.UserID,
+			HelpFlags1: cmd.HelpFlags1,
+			Updated:    sqlstore.TimeNow(),
+		}
+
+		_, err := sess.ID(cmd.UserID).Cols("help_flags1").Update(&user)
+		return err
+	})
+}
+
+// UpdatePermissions sets the user Server Admin flag
+func (ss *sqlStore) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error {
+	return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		var user user.User
+		if _, err := sess.ID(userID).Where(ss.notServiceAccountFilter()).Get(&user); err != nil {
+			return err
+		}
+
+		user.IsAdmin = isAdmin
+		sess.UseBool("is_admin")
+		_, err := sess.ID(user.ID).Update(&user)
+		if err != nil {
+			return err
+		}
+		// validate that after update there is at least one server admin
+		if err := validateOneAdminLeft(ctx, sess); err != nil {
+			return err
+		}
+		return nil
+	})
+}
+
+// validateOneAdminLeft validate that there is an admin user left
+func validateOneAdminLeft(ctx context.Context, sess *sqlstore.DBSession) error {
+	count, err := sess.Where("is_admin=?", true).Count(&user.User{})
+	if err != nil {
+		return err
+	}
+
+	if count == 0 {
+		return user.ErrLastGrafanaAdmin
+	}
+
+	return nil
+}

pkg/services/user/userimpl/store_test.go

@@ -246,6 +246,71 @@ func TestIntegrationUserDataAccess(t *testing.T) {
 		assert.Equal(t, result.Name, "testtestest")
 		assert.Equal(t, result.Login, "loginloginlogin")
 	})
+
+	t.Run("Testing DB - grafana admin users", func(t *testing.T) {
+		ss = sqlstore.InitTestDB(t)
+
+		createUserCmd := user.CreateUserCommand{
+			Email:   fmt.Sprint("admin", "@test.com"),
+			Name:    "admin",
+			Login:   "admin",
+			IsAdmin: true,
+		}
+		usr, err := ss.CreateUser(context.Background(), createUserCmd)
+		require.Nil(t, err)
+
+		// Cannot make themselves a non-admin
+		updatePermsError := userStore.UpdatePermissions(context.Background(), usr.ID, false)
+
+		require.Equal(t, user.ErrLastGrafanaAdmin, updatePermsError)
+
+		query := models.GetUserByIdQuery{Id: usr.ID}
+		getUserError := ss.GetUserById(context.Background(), &query)
+		require.Nil(t, getUserError)
+
+		require.True(t, query.Result.IsAdmin)
+
+		// One user
+		const email = "user@test.com"
+		const username = "user"
+		createUserCmd = user.CreateUserCommand{
+			Email: email,
+			Name:  "user",
+			Login: username,
+		}
+		_, err = ss.CreateUser(context.Background(), createUserCmd)
+		require.Nil(t, err)
+
+		// When trying to create a new user with the same email, an error is returned
+		createUserCmd = user.CreateUserCommand{
+			Email:        email,
+			Name:         "user2",
+			Login:        "user2",
+			SkipOrgSetup: true,
+		}
+		_, err = ss.CreateUser(context.Background(), createUserCmd)
+		require.Equal(t, err, user.ErrUserAlreadyExists)
+
+		// When trying to create a new user with the same login, an error is returned
+		createUserCmd = user.CreateUserCommand{
+			Email:        "user2@test.com",
+			Name:         "user2",
+			Login:        username,
+			SkipOrgSetup: true,
+		}
+		_, err = ss.CreateUser(context.Background(), createUserCmd)
+		require.Equal(t, err, user.ErrUserAlreadyExists)
+	})
+
+	t.Run("GetProfile", func(t *testing.T) {
+		_, err := userStore.GetProfile(context.Background(), &user.GetUserProfileQuery{UserID: 1})
+		require.NoError(t, err)
+	})
+
+	t.Run("SetHelpFlag", func(t *testing.T) {
+		err := userStore.SetHelpFlag(context.Background(), &user.SetUserHelpFlagCommand{UserID: 1, HelpFlags1: user.HelpFlags1(1)})
+		require.NoError(t, err)
+	})
 }
 
 func TestIntegrationUserUpdate(t *testing.T) {

pkg/services/user/userimpl/user.go

@@ -319,44 +319,15 @@ func (s *Service) BatchDisableUsers(ctx context.Context, cmd *user.BatchDisableU
 	return s.sqlStore.BatchDisableUsers(ctx, c)
 }
 
-// TODO: remove wrapper around sqlstore
+func (s *Service) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error {
-func (s *Service) UpdatePermissions(userID int64, isAdmin bool) error {
+	return s.store.UpdatePermissions(ctx, userID, isAdmin)
-	return s.sqlStore.UpdateUserPermissions(userID, isAdmin)
 }
 
-// TODO: remove wrapper around sqlstore
 func (s *Service) SetUserHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCommand) error {
-	c := &models.SetUserHelpFlagCommand{
+	return s.store.SetHelpFlag(ctx, cmd)
-		UserId:     cmd.UserID,
-		HelpFlags1: cmd.HelpFlags1,
-	}
-	return s.sqlStore.SetUserHelpFlag(ctx, c)
 }
 
-// TODO: remove wrapper around sqlstore
+func (s *Service) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) {
-func (s *Service) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (user.UserProfileDTO, error) {
+	result, err := s.store.GetProfile(ctx, query)
-	q := &models.GetUserProfileQuery{
+	return result, err
-		UserId: query.UserID,
-	}
-	err := s.sqlStore.GetUserProfile(ctx, q)
-	if err != nil {
-		return user.UserProfileDTO{}, err
-	}
-	result := user.UserProfileDTO{
-		ID:             q.Result.Id,
-		Email:          q.Result.Email,
-		Name:           q.Result.Name,
-		Login:          q.Result.Login,
-		Theme:          q.Result.Theme,
-		OrgID:          q.Result.OrgId,
-		IsGrafanaAdmin: q.Result.IsGrafanaAdmin,
-		IsDisabled:     q.Result.IsDisabled,
-		IsExternal:     q.Result.IsExternal,
-		AuthLabels:     q.Result.AuthLabels,
-		UpdatedAt:      q.Result.UpdatedAt,
-		CreatedAt:      q.Result.CreatedAt,
-		AvatarUrl:      q.Result.AvatarUrl,
-		AccessControl:  q.Result.AccessControl,
-	}
-	return result, nil
 }

pkg/services/user/userimpl/user_test.go

@@ -131,6 +131,7 @@ func TestUserService(t *testing.T) {
 type FakeUserStore struct {
 	ExpectedUser            *user.User
 	ExpectedSignedInUser    *user.SignedInUser
+	ExpectedUserProfile     *user.UserProfileDTO
 	ExpectedError           error
 	ExpectedDeleteUserError error
 }
@@ -190,3 +191,15 @@ func (f *FakeUserStore) GetSignedInUser(ctx context.Context, query *user.GetSign
 func (f *FakeUserStore) UpdateUser(ctx context.Context, user *user.User) error {
 	return f.ExpectedError
 }
+
+func (f *FakeUserStore) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) {
+	return f.ExpectedUserProfile, f.ExpectedError
+}
+
+func (f *FakeUserStore) SetHelpFlag(ctx context.Context, cmd *user.SetUserHelpFlagCommand) error {
+	return f.ExpectedError
+}
+
+func (f *FakeUserStore) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error {
+	return f.ExpectedError
+}

pkg/services/user/usertest/fake.go

@@ -12,7 +12,7 @@ type FakeUserService struct {
 	ExpectedError            error
 	ExpectedSetUsingOrgError error
 	ExpectedSearchUsers      user.SearchUserQueryResult
-	ExpectedUSerProfileDTO   user.UserProfileDTO
+	ExpectedUserProfileDTO   *user.UserProfileDTO
 
 	GetSignedInUserFn func(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error)
 }
@@ -83,7 +83,7 @@ func (f *FakeUserService) BatchDisableUsers(ctx context.Context, cmd *user.Batch
 	return f.ExpectedError
 }
 
-func (f *FakeUserService) UpdatePermissions(userID int64, isAdmin bool) error {
+func (f *FakeUserService) UpdatePermissions(ctx context.Context, userID int64, isAdmin bool) error {
 	return f.ExpectedError
 }
 
@@ -91,6 +91,6 @@ func (f *FakeUserService) SetUserHelpFlag(ctx context.Context, cmd *user.SetUser
 	return f.ExpectedError
 }
 
-func (f *FakeUserService) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (user.UserProfileDTO, error) {
+func (f *FakeUserService) GetProfile(ctx context.Context, query *user.GetUserProfileQuery) (*user.UserProfileDTO, error) {
-	return f.ExpectedUSerProfileDTO, f.ExpectedError
+	return f.ExpectedUserProfileDTO, f.ExpectedError
 }