Chore: Use AddUserOrg from org service (#55657)

* Chore: Copy methods from sqlstore to org store

* Rename method, add test

* Add comments of tests

* Chore: Add methods from sqlstore to org service interface

* Avoiding import cycle

* Add and remove some methods

* User AddOrgUSer from org service in api

* Fix test function calls
This commit is contained in:
idafurjes 2022-09-23 11:59:07 +02:00 committed by GitHub
parent 96cb415497
commit 883c7a802b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 40 additions and 26 deletions

View File

@ -13,6 +13,7 @@ import (
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/models"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
@ -141,8 +142,8 @@ func (hs *HTTPServer) AddOrgInvite(c *models.ReqContext) response.Response {
func (hs *HTTPServer) inviteExistingUserToOrg(c *models.ReqContext, user *user.User, inviteDto *dtos.AddInviteForm) response.Response {
// user exists, add org role
createOrgUserCmd := models.AddOrgUserCommand{OrgId: c.OrgID, UserId: user.ID, Role: inviteDto.Role}
if err := hs.SQLStore.AddOrgUser(c.Req.Context(), &createOrgUserCmd); err != nil {
createOrgUserCmd := org.AddOrgUserCommand{OrgID: c.OrgID, UserID: user.ID, Role: inviteDto.Role}
if err := hs.orgService.AddOrgUser(c.Req.Context(), &createOrgUserCmd); err != nil {
if errors.Is(err, models.ErrOrgUserAlreadyAdded) {
return response.Error(412, fmt.Sprintf("User %s is already added to organization", inviteDto.LoginOrEmail), err)
}
@ -287,8 +288,8 @@ func (hs *HTTPServer) updateTempUserStatus(ctx context.Context, code string, sta
func (hs *HTTPServer) applyUserInvite(ctx context.Context, usr *user.User, invite *models.TempUserDTO, setActive bool) (bool, response.Response) {
// add to org
addOrgUserCmd := models.AddOrgUserCommand{OrgId: invite.OrgId, UserId: usr.ID, Role: invite.Role}
if err := hs.SQLStore.AddOrgUser(ctx, &addOrgUserCmd); err != nil {
addOrgUserCmd := org.AddOrgUserCommand{OrgID: invite.OrgId, UserID: usr.ID, Role: invite.Role}
if err := hs.orgService.AddOrgUser(ctx, &addOrgUserCmd); err != nil {
if !errors.Is(err, models.ErrOrgUserAlreadyAdded) {
return false, response.Error(500, "Error while trying to create org user", err)
}

View File

@ -11,6 +11,7 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
@ -31,11 +32,11 @@ import (
// 403: forbiddenError
// 500: internalServerError
func (hs *HTTPServer) AddOrgUserToCurrentOrg(c *models.ReqContext) response.Response {
cmd := models.AddOrgUserCommand{}
cmd := org.AddOrgUserCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
cmd.OrgId = c.OrgID
cmd.OrgID = c.OrgID
return hs.addOrgUserHelper(c, cmd)
}
@ -54,20 +55,20 @@ func (hs *HTTPServer) AddOrgUserToCurrentOrg(c *models.ReqContext) response.Resp
// 403: forbiddenError
// 500: internalServerError
func (hs *HTTPServer) AddOrgUser(c *models.ReqContext) response.Response {
cmd := models.AddOrgUserCommand{}
cmd := org.AddOrgUserCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
var err error
cmd.OrgId, err = strconv.ParseInt(web.Params(c.Req)[":orgId"], 10, 64)
cmd.OrgID, err = strconv.ParseInt(web.Params(c.Req)[":orgId"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "orgId is invalid", err)
}
return hs.addOrgUserHelper(c, cmd)
}
func (hs *HTTPServer) addOrgUserHelper(c *models.ReqContext, cmd models.AddOrgUserCommand) response.Response {
func (hs *HTTPServer) addOrgUserHelper(c *models.ReqContext, cmd org.AddOrgUserCommand) response.Response {
if !cmd.Role.IsValid() {
return response.Error(400, "Invalid role specified", nil)
}
@ -81,13 +82,13 @@ func (hs *HTTPServer) addOrgUserHelper(c *models.ReqContext, cmd models.AddOrgUs
return response.Error(404, "User not found", nil)
}
cmd.UserId = userToAdd.ID
cmd.UserID = userToAdd.ID
if err := hs.SQLStore.AddOrgUser(c.Req.Context(), &cmd); err != nil {
if err := hs.orgService.AddOrgUser(c.Req.Context(), &cmd); err != nil {
if errors.Is(err, models.ErrOrgUserAlreadyAdded) {
return response.JSON(409, util.DynMap{
"message": "User is already member of this organization",
"userId": cmd.UserId,
"userId": cmd.UserID,
})
}
return response.Error(500, "Could not add user to organization", err)
@ -95,7 +96,7 @@ func (hs *HTTPServer) addOrgUserHelper(c *models.ReqContext, cmd models.AddOrgUs
return response.JSON(http.StatusOK, util.DynMap{
"message": "User added to organization",
"userId": cmd.UserId,
"userId": cmd.UserID,
})
}

View File

@ -8,6 +8,7 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
@ -23,6 +24,7 @@ func ProvideService(
quotaService quota.Service,
authInfoService login.AuthInfoService,
accessControl accesscontrol.Service,
orgService org.Service,
) *Implementation {
s := &Implementation{
SQLStore: sqlStore,
@ -30,6 +32,7 @@ func ProvideService(
QuotaService: quotaService,
AuthInfoService: authInfoService,
accessControl: accessControl,
orgService: orgService,
}
return s
}
@ -41,6 +44,7 @@ type Implementation struct {
QuotaService quota.Service
TeamSync login.TeamSyncFunc
accessControl accesscontrol.Service
orgService org.Service
}
// CreateUser creates inserts a new one.
@ -298,8 +302,8 @@ func (ls *Implementation) syncOrgRoles(ctx context.Context, usr *user.User, extU
}
// add role
cmd := &models.AddOrgUserCommand{UserId: usr.ID, Role: orgRole, OrgId: orgId}
err := ls.SQLStore.AddOrgUser(ctx, cmd)
cmd := &org.AddOrgUserCommand{UserID: usr.ID, Role: orgRole, OrgID: orgId}
err := ls.orgService.AddOrgUser(ctx, cmd)
if err != nil && !errors.Is(err, models.ErrOrgNotFound) {
return err
}

View File

@ -22,6 +22,7 @@ import (
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
"github.com/grafana/grafana/pkg/services/licensing"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
@ -43,7 +44,8 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) {
store := sqlstore.InitTestDB(t)
apiKeyService := apikeyimpl.ProvideService(store, store.Cfg)
kvStore := kvstore.ProvideService(store)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore)
orgService := orgimpl.ProvideService(store, setting.NewCfg())
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, orgService)
svcmock := tests.ServiceAccountMock{}
autoAssignOrg := store.Cfg.AutoAssignOrg
@ -209,7 +211,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
store := sqlstore.InitTestDB(t)
kvStore := kvstore.ProvideService(store)
apiKeyService := apikeyimpl.ProvideService(store, store.Cfg)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
var requestResponse = func(server *web.Mux, httpMethod, requestpath string) *httptest.ResponseRecorder {
@ -312,7 +314,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) {
store := sqlstore.InitTestDB(t)
apiKeyService := apikeyimpl.ProvideService(store, store.Cfg)
kvStore := kvstore.ProvideService(store)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
type testRetrieveSATestCase struct {
desc string
@ -404,7 +406,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
store := sqlstore.InitTestDB(t)
apiKeyService := apikeyimpl.ProvideService(store, store.Cfg)
kvStore := kvstore.ProvideService(store)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
type testUpdateSATestCase struct {
desc string

View File

@ -55,7 +55,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
store := sqlstore.InitTestDB(t)
apiKeyService := apikeyimpl.ProvideService(store, store.Cfg)
kvStore := kvstore.ProvideService(store)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
svcmock := tests.ServiceAccountMock{}
sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
@ -173,7 +173,7 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
apiKeyService := apikeyimpl.ProvideService(store, store.Cfg)
kvStore := kvstore.ProvideService(store)
svcMock := &tests.ServiceAccountMock{}
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore)
saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore, nil)
sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
type testCreateSAToken struct {

View File

@ -25,14 +25,17 @@ type ServiceAccountsStoreImpl struct {
kvStore kvstore.KVStore
log log.Logger
userService user.Service
orgService org.Service
}
func ProvideServiceAccountsStore(store *sqlstore.SQLStore, apiKeyService apikey.Service, kvStore kvstore.KVStore) *ServiceAccountsStoreImpl {
func ProvideServiceAccountsStore(store *sqlstore.SQLStore, apiKeyService apikey.Service,
kvStore kvstore.KVStore, orgService org.Service) *ServiceAccountsStoreImpl {
return &ServiceAccountsStoreImpl{
sqlStore: store,
apiKeyService: apiKeyService,
kvStore: kvStore,
log: log.New("serviceaccounts.store"),
orgService: orgService,
}
}
@ -63,10 +66,10 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org
return errUser
}
errAddOrgUser := s.sqlStore.AddOrgUser(ctx, &models.AddOrgUserCommand{
errAddOrgUser := s.orgService.AddOrgUser(ctx, &org.AddOrgUserCommand{
Role: role,
OrgId: orgId,
UserId: newSA.ID,
OrgID: orgId,
UserID: newSA.ID,
AllowAddingServiceAccount: true,
})
if errAddOrgUser != nil {

View File

@ -9,10 +9,12 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/apikey/apikeyimpl"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -110,7 +112,8 @@ func setupTestDatabase(t *testing.T) (*sqlstore.SQLStore, *ServiceAccountsStoreI
db := sqlstore.InitTestDB(t)
apiKeyService := apikeyimpl.ProvideService(db, db.Cfg)
kvStore := kvstore.ProvideService(db)
return db, ProvideServiceAccountsStore(db, apiKeyService, kvStore)
orgService := orgimpl.ProvideService(db, setting.NewCfg())
return db, ProvideServiceAccountsStore(db, apiKeyService, kvStore, orgService)
}
func TestStore_RetrieveServiceAccount(t *testing.T) {