Chore: Use org service methods (#55738)

* Chore: Use org service methods

* Fix loginservice test

* User Serach from org service

* Fix test
This commit is contained in:
idafurjes 2022-09-26 18:53:17 +02:00 committed by GitHub
parent 29fdbf0354
commit 846a4510b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 64 additions and 28 deletions

View File

@ -246,7 +246,7 @@ func (hs *HTTPServer) SearchOrgUsersWithPaging(c *models.ReqContext) response.Re
page = 1
}
query := &models.SearchOrgUsersQuery{
query := &org.SearchOrgUsersQuery{
OrgID: c.OrgID,
Query: c.Query("query"),
Page: page,
@ -254,25 +254,26 @@ func (hs *HTTPServer) SearchOrgUsersWithPaging(c *models.ReqContext) response.Re
User: c.SignedInUser,
}
if err := hs.SQLStore.SearchOrgUsers(ctx, query); err != nil {
result, err := hs.orgService.SearchOrgUsers(ctx, query)
if err != nil {
return response.Error(500, "Failed to get users for current organization", err)
}
filteredUsers := make([]*models.OrgUserDTO, 0, len(query.Result.OrgUsers))
for _, user := range query.Result.OrgUsers {
filteredUsers := make([]*org.OrgUserDTO, 0, len(result.OrgUsers))
for _, user := range result.OrgUsers {
if dtos.IsHiddenUser(user.Login, c.SignedInUser, hs.Cfg) {
continue
}
user.AvatarUrl = dtos.GetGravatarUrl(user.Email)
user.AvatarURL = dtos.GetGravatarUrl(user.Email)
filteredUsers = append(filteredUsers, user)
}
query.Result.OrgUsers = filteredUsers
query.Result.Page = page
query.Result.PerPage = perPage
result.OrgUsers = filteredUsers
result.Page = page
result.PerPage = perPage
return response.JSON(http.StatusOK, query.Result)
return response.JSON(http.StatusOK, result)
}
// swagger:route PATCH /org/users/{user_id} org updateOrgUserForCurrentOrg
@ -368,9 +369,9 @@ func (hs *HTTPServer) RemoveOrgUserForCurrentOrg(c *models.ReqContext) response.
return response.Error(http.StatusBadRequest, "userId is invalid", err)
}
return hs.removeOrgUserHelper(c.Req.Context(), &models.RemoveOrgUserCommand{
UserId: userId,
OrgId: c.OrgID,
return hs.removeOrgUserHelper(c.Req.Context(), &org.RemoveOrgUserCommand{
UserID: userId,
OrgID: c.OrgID,
ShouldDeleteOrphanedUser: true,
})
}
@ -397,14 +398,14 @@ func (hs *HTTPServer) RemoveOrgUser(c *models.ReqContext) response.Response {
if err != nil {
return response.Error(http.StatusBadRequest, "orgId is invalid", err)
}
return hs.removeOrgUserHelper(c.Req.Context(), &models.RemoveOrgUserCommand{
UserId: userId,
OrgId: orgId,
return hs.removeOrgUserHelper(c.Req.Context(), &org.RemoveOrgUserCommand{
UserID: userId,
OrgID: orgId,
})
}
func (hs *HTTPServer) removeOrgUserHelper(ctx context.Context, cmd *models.RemoveOrgUserCommand) response.Response {
if err := hs.SQLStore.RemoveOrgUser(ctx, cmd); err != nil {
func (hs *HTTPServer) removeOrgUserHelper(ctx context.Context, cmd *org.RemoveOrgUserCommand) response.Response {
if err := hs.orgService.RemoveOrgUser(ctx, cmd); err != nil {
if errors.Is(err, models.ErrLastOrgAdmin) {
return response.Error(400, "Cannot remove last organization admin", nil)
}
@ -413,15 +414,15 @@ func (hs *HTTPServer) removeOrgUserHelper(ctx context.Context, cmd *models.Remov
if cmd.UserWasDeleted {
// This should be called from appropriate service when moved
if err := hs.accesscontrolService.DeleteUserPermissions(ctx, accesscontrol.GlobalOrgID, cmd.UserId); err != nil {
hs.log.Warn("failed to delete permissions for user", "userID", cmd.UserId, "orgID", accesscontrol.GlobalOrgID, "err", err)
if err := hs.accesscontrolService.DeleteUserPermissions(ctx, accesscontrol.GlobalOrgID, cmd.UserID); err != nil {
hs.log.Warn("failed to delete permissions for user", "userID", cmd.UserID, "orgID", accesscontrol.GlobalOrgID, "err", err)
}
return response.Success("User deleted")
}
// This should be called from appropriate service when moved
if err := hs.accesscontrolService.DeleteUserPermissions(ctx, cmd.OrgId, cmd.UserId); err != nil {
hs.log.Warn("failed to delete permissions for user", "userID", cmd.UserId, "orgID", cmd.OrgId, "err", err)
if err := hs.accesscontrolService.DeleteUserPermissions(ctx, cmd.OrgID, cmd.UserID); err != nil {
hs.log.Warn("failed to delete permissions for user", "userID", cmd.UserID, "orgID", cmd.OrgID, "err", err)
}
return response.Success("User removed from organization")

View File

@ -49,6 +49,7 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
sqlStore.Cfg = settings
hs.SQLStore = sqlStore
orgService := orgtest.NewOrgServiceFake()
orgService.ExpectedSearchOrgUsersResult = &org.SearchOrgUsersQueryResult{}
hs.orgService = orgService
mock := mockstore.NewSQLStoreMock()
loggedInUserScenario(t, "When calling GET on", "api/org/users", "api/org/users", func(sc *scenarioContext) {
@ -72,12 +73,28 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
loggedInUserScenario(t, "When calling GET on", "api/org/users/search", "api/org/users/search", func(sc *scenarioContext) {
setUpGetOrgUsersDB(t, sqlStore)
orgService.ExpectedSearchOrgUsersResult = &org.SearchOrgUsersQueryResult{
OrgUsers: []*org.OrgUserDTO{
{
Login: "user1",
},
{
Login: "user2",
},
{
Login: "user3",
},
},
TotalCount: 3,
PerPage: 1000,
Page: 1,
}
sc.handlerFunc = hs.SearchOrgUsersWithPaging
sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
require.Equal(t, http.StatusOK, sc.resp.Code)
var resp models.SearchOrgUsersQueryResult
var resp org.SearchOrgUsersQueryResult
err := json.Unmarshal(sc.resp.Body.Bytes(), &resp)
require.NoError(t, err)
@ -90,12 +107,23 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
loggedInUserScenario(t, "When calling GET with page and limit query parameters on", "api/org/users/search", "api/org/users/search", func(sc *scenarioContext) {
setUpGetOrgUsersDB(t, sqlStore)
orgService.ExpectedSearchOrgUsersResult = &org.SearchOrgUsersQueryResult{
OrgUsers: []*org.OrgUserDTO{
{
Login: "user1",
},
},
TotalCount: 3,
PerPage: 2,
Page: 2,
}
sc.handlerFunc = hs.SearchOrgUsersWithPaging
sc.fakeReqWithParams("GET", sc.url, map[string]string{"perpage": "2", "page": "2"}).exec()
require.Equal(t, http.StatusOK, sc.resp.Code)
var resp models.SearchOrgUsersQueryResult
var resp org.SearchOrgUsersQueryResult
err := json.Unmarshal(sc.resp.Body.Bytes(), &resp)
require.NoError(t, err)
@ -947,6 +975,7 @@ func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
hs.SQLStore, nil, nil, nil, nil,
nil, nil, nil, nil, nil, hs.SQLStore.(*sqlstore.SQLStore),
)
hs.orgService = orgimpl.ProvideService(hs.SQLStore, cfg)
})
setupOrgUsersDBForAccessControlTests(t, sc.db)
setInitCtxSignedInUser(sc.initCtx, tc.user)

View File

@ -313,14 +313,14 @@ func (ls *Implementation) syncOrgRoles(ctx context.Context, usr *user.User, extU
for _, orgId := range deleteOrgIds {
logger.Debug("Removing user's organization membership as part of syncing with OAuth login",
"userId", usr.ID, "orgId", orgId)
cmd := &models.RemoveOrgUserCommand{OrgId: orgId, UserId: usr.ID}
if err := ls.SQLStore.RemoveOrgUser(ctx, cmd); err != nil {
cmd := &org.RemoveOrgUserCommand{OrgID: orgId, UserID: usr.ID}
if err := ls.orgService.RemoveOrgUser(ctx, cmd); err != nil {
if errors.Is(err, models.ErrLastOrgAdmin) {
logger.Error(err.Error(), "userId", cmd.UserId, "orgId", cmd.OrgId)
logger.Error(err.Error(), "userId", cmd.UserID, "orgId", cmd.OrgID)
continue
}
if err := ls.accessControl.DeleteUserPermissions(ctx, orgId, cmd.UserId); err != nil {
logger.Warn("failed to delete permissions for user", "userID", cmd.UserId, "orgID", orgId)
if err := ls.accessControl.DeleteUserPermissions(ctx, orgId, cmd.UserID); err != nil {
logger.Warn("failed to delete permissions for user", "userID", cmd.UserID, "orgID", orgId)
}
return err

View File

@ -12,6 +12,7 @@ import (
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/login/logintest"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/services/user"
@ -35,6 +36,7 @@ func Test_syncOrgRoles_doesNotBreakWhenTryingToRemoveLastOrgAdmin(t *testing.T)
AuthInfoService: authInfoMock,
SQLStore: store,
userService: usertest.NewUserServiceFake(),
orgService: orgtest.NewOrgServiceFake(),
}
err := login.syncOrgRoles(context.Background(), &user, &externalUser)
@ -55,11 +57,15 @@ func Test_syncOrgRoles_whenTryingToRemoveLastOrgLogsError(t *testing.T) {
ExpectedOrgListResponse: createResponseWithOneErrLastOrgAdminItem(),
}
orgService := orgtest.NewOrgServiceFake()
orgService.ExpectedError = models.ErrLastOrgAdmin
login := Implementation{
QuotaService: &quotaimpl.Service{},
AuthInfoService: authInfoMock,
SQLStore: store,
userService: usertest.NewUserServiceFake(),
orgService: orgService,
}
err := login.syncOrgRoles(context.Background(), &user, &externalUser)