Chore: Move updateorg out of sqlstore (#54111)

* Chore: move updateorg out of sqlstore

* fix api test

pkg/api/http_server.go

@@ -59,6 +59,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/ngalert"
 	"github.com/grafana/grafana/pkg/services/notifications"
+	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/playlist"
 	"github.com/grafana/grafana/pkg/services/plugindashboards"
 	pluginSettings "github.com/grafana/grafana/pkg/services/pluginsettings/service"
@@ -180,6 +181,7 @@ type HTTPServer struct {
 	userService                  user.Service
 	tempUserService              tempUser.Service
 	loginAttemptService          loginAttempt.Service
+	orgService                   org.Service
 }
 
 type ServerOptions struct {
@@ -215,7 +217,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardVersionService dashver.Service,
 	starService star.Service, csrfService csrf.Service, coremodels *registry.Base,
 	playlistService playlist.Service, apiKeyService apikey.Service, kvStore kvstore.KVStore, secretsMigrator secrets.Migrator, secretsPluginManager plugins.SecretsPluginManager,
-	publicDashboardsApi *publicdashboardsApi.Api, userService user.Service, tempUserService tempUser.Service, loginAttemptService loginAttempt.Service) (*HTTPServer, error) {
+	publicDashboardsApi *publicdashboardsApi.Api, userService user.Service, tempUserService tempUser.Service, loginAttemptService loginAttempt.Service, orgService org.Service) (*HTTPServer, error) {
 	web.Env = cfg.Env
 	m := web.New()
 
@@ -305,6 +307,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 		userService:                  userService,
 		tempUserService:              tempUserService,
 		loginAttemptService:          loginAttemptService,
+		orgService:                   orgService,
 	}
 	if hs.Listener != nil {
 		hs.log.Debug("Using provided listener")

pkg/api/org.go

@@ -10,6 +10,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
@@ -194,8 +195,8 @@ func (hs *HTTPServer) UpdateOrg(c *models.ReqContext) response.Response {
 }
 
 func (hs *HTTPServer) updateOrgHelper(ctx context.Context, form dtos.UpdateOrgForm, orgID int64) response.Response {
-	cmd := models.UpdateOrgCommand{Name: form.Name, OrgId: orgID}
-	if err := hs.SQLStore.UpdateOrg(ctx, &cmd); err != nil {
+	cmd := org.UpdateOrgCommand{Name: form.Name, OrgId: orgID}
+	if err := hs.orgService.UpdateOrg(ctx, &cmd); err != nil {
 		if errors.Is(err, models.ErrOrgNameTaken) {
 			return response.Error(http.StatusBadRequest, "Organization name taken", err)
 		}

pkg/api/org_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/org/orgimpl"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 )
@@ -102,6 +103,7 @@ func TestAPIEndpoint_PutCurrentOrg_LegacyAccessControl(t *testing.T) {
 	})
 
 	setInitCtxSignedInOrgAdmin(sc.initCtx)
+	sc.hs.orgService = orgimpl.ProvideService(sc.db, sc.cfg)
 	t.Run("Admin can update current org", func(t *testing.T) {
 		response := callAPI(sc.server, http.MethodPut, putCurrentOrgURL, input, t)
 		assert.Equal(t, http.StatusOK, response.Code)
@@ -115,6 +117,8 @@ func TestAPIEndpoint_PutCurrentOrg_AccessControl(t *testing.T) {
 	_, err := sc.db.CreateOrgWithMember("TestOrg", sc.initCtx.UserID)
 	require.NoError(t, err)
 
+	sc.hs.orgService = orgimpl.ProvideService(sc.db, sc.cfg)
+
 	input := strings.NewReader(testUpdateOrgNameForm)
 	t.Run("AccessControl allows updating current org with correct permissions", func(t *testing.T) {
 		setAccessControlPermissions(sc.acmock, []accesscontrol.Permission{{Action: ActionOrgsWrite}}, sc.initCtx.OrgID)
@@ -423,7 +427,7 @@ func TestAPIEndpoint_PutOrg_LegacyAccessControl(t *testing.T) {
 	cfg.RBACEnabled = false
 	sc := setupHTTPServerWithCfg(t, true, cfg)
 	setInitCtxSignedInViewer(sc.initCtx)
-
+	sc.hs.orgService = orgimpl.ProvideService(sc.db, sc.cfg)
 	// Create two orgs, to update another one than the logged in one
 	setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
 
@@ -444,7 +448,7 @@ func TestAPIEndpoint_PutOrg_LegacyAccessControl(t *testing.T) {
 func TestAPIEndpoint_PutOrg_AccessControl(t *testing.T) {
 	sc := setupHTTPServer(t, true)
 	setInitCtxSignedInViewer(sc.initCtx)
-
+	sc.hs.orgService = orgimpl.ProvideService(sc.db, sc.cfg)
 	// Create two orgs, to update another one than the logged in one
 	setupOrgsDBForAccessControlTests(t, sc.db, sc, 2)
 

pkg/models/org.go

@@ -44,11 +44,6 @@ type DeleteOrgCommand struct {
 	Id int64
 }
 
-type UpdateOrgCommand struct {
-	Name  string
-	OrgId int64
-}
-
 type UpdateOrgAddressCommand struct {
 	OrgId int64
 	Address

pkg/services/org/model.go

@@ -71,6 +71,11 @@ type UserOrgDTO struct {
 	Role  RoleType `json:"role"`
 }
 
+type UpdateOrgCommand struct {
+	Name  string
+	OrgId int64
+}
+
 func (r RoleType) IsValid() bool {
 	return r == RoleViewer || r == RoleAdmin || r == RoleEditor
 }

pkg/services/org/org.go

@@ -9,4 +9,5 @@ type Service interface {
 	InsertOrgUser(context.Context, *OrgUser) (int64, error)
 	DeleteUserFromAll(context.Context, int64) error
 	GetUserOrgList(context.Context, *GetUserOrgListQuery) ([]*UserOrgDTO, error)
+	UpdateOrg(ctx context.Context, cmd *UpdateOrgCommand) error
 }

pkg/services/org/orgimpl/org.go

@@ -104,3 +104,7 @@ func (s *Service) GetUserOrgList(ctx context.Context, query *org.GetUserOrgListQ
 	}
 	return result, nil
 }
+
+func (s *Service) UpdateOrg(ctx context.Context, cmd *org.UpdateOrgCommand) error {
+	return s.store.Update(ctx, cmd)
+}

pkg/services/org/orgimpl/org_test.go

@@ -79,3 +79,7 @@ func (f *FakeOrgStore) InsertOrgUser(ctx context.Context, org *org.OrgUser) (int
 func (f *FakeOrgStore) DeleteUserFromAll(ctx context.Context, userID int64) error {
 	return f.ExpectedError
 }
+
+func (f *FakeOrgStore) Update(ctx context.Context, cmd *org.UpdateOrgCommand) error {
+	return f.ExpectedError
+}

pkg/services/org/orgimpl/store.go

@@ -2,8 +2,10 @@ package orgimpl
 
 import (
 	"context"
+	"time"
 
 	"github.com/grafana/grafana/pkg/events"
+	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/services/sqlstore/db"
@@ -17,6 +19,7 @@ type store interface {
 	Insert(context.Context, *org.Org) (int64, error)
 	InsertOrgUser(context.Context, *org.OrgUser) (int64, error)
 	DeleteUserFromAll(context.Context, int64) error
+	Update(ctx context.Context, cmd *org.UpdateOrgCommand) error
 }
 
 type sqlStore struct {
@@ -91,3 +94,52 @@ func (ss *sqlStore) DeleteUserFromAll(ctx context.Context, userID int64) error {
 		return nil
 	})
 }
+
+func (ss *sqlStore) Update(ctx context.Context, cmd *org.UpdateOrgCommand) error {
+	return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		if isNameTaken, err := isOrgNameTaken(cmd.Name, cmd.OrgId, sess); err != nil {
+			return err
+		} else if isNameTaken {
+			return models.ErrOrgNameTaken
+		}
+
+		org := models.Org{
+			Name:    cmd.Name,
+			Updated: time.Now(),
+		}
+
+		affectedRows, err := sess.ID(cmd.OrgId).Update(&org)
+
+		if err != nil {
+			return err
+		}
+
+		if affectedRows == 0 {
+			return models.ErrOrgNotFound
+		}
+
+		sess.PublishAfterCommit(&events.OrgUpdated{
+			Timestamp: org.Updated,
+			Id:        org.Id,
+			Name:      org.Name,
+		})
+
+		return nil
+	})
+}
+
+func isOrgNameTaken(name string, existingId int64, sess *sqlstore.DBSession) (bool, error) {
+	// check if org name is taken
+	var org models.Org
+	exists, err := sess.Where("name=?", name).Get(&org)
+
+	if err != nil {
+		return false, nil
+	}
+
+	if exists && existingId != org.Id {
+		return true, nil
+	}
+
+	return false, nil
+}

pkg/services/org/orgtest/fake.go

@@ -35,3 +35,7 @@ func (f *FakeOrgService) DeleteUserFromAll(ctx context.Context, userID int64) er
 func (f *FakeOrgService) GetUserOrgList(ctx context.Context, query *org.GetUserOrgListQuery) ([]*org.UserOrgDTO, error) {
 	return f.ExpectedUserOrgDTO, f.ExpectedError
 }
+
+func (f *FakeOrgService) UpdateOrg(ctx context.Context, cmd *org.UpdateOrgCommand) error {
+	return f.ExpectedError
+}

pkg/services/sqlstore/mockstore/mockstore.go

@@ -100,10 +100,6 @@ func (m *SQLStoreMock) CreateOrg(ctx context.Context, cmd *models.CreateOrgComma
 	return m.ExpectedError
 }
 
-func (m *SQLStoreMock) UpdateOrg(ctx context.Context, cmd *models.UpdateOrgCommand) error {
-	return m.ExpectedError
-}
-
 func (m *SQLStoreMock) UpdateOrgAddress(ctx context.Context, cmd *models.UpdateOrgAddressCommand) error {
 	return m.ExpectedError
 }

pkg/services/sqlstore/org.go

@@ -159,39 +159,6 @@ func (ss *SQLStore) CreateOrg(ctx context.Context, cmd *models.CreateOrgCommand)
 	return nil
 }
 
-func (ss *SQLStore) UpdateOrg(ctx context.Context, cmd *models.UpdateOrgCommand) error {
-	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
-		if isNameTaken, err := isOrgNameTaken(cmd.Name, cmd.OrgId, sess); err != nil {
-			return err
-		} else if isNameTaken {
-			return models.ErrOrgNameTaken
-		}
-
-		org := models.Org{
-			Name:    cmd.Name,
-			Updated: time.Now(),
-		}
-
-		affectedRows, err := sess.ID(cmd.OrgId).Update(&org)
-
-		if err != nil {
-			return err
-		}
-
-		if affectedRows == 0 {
-			return models.ErrOrgNotFound
-		}
-
-		sess.publishAfterCommit(&events.OrgUpdated{
-			Timestamp: org.Updated,
-			Id:        org.Id,
-			Name:      org.Name,
-		})
-
-		return nil
-	})
-}
-
 func (ss *SQLStore) UpdateOrgAddress(ctx context.Context, cmd *models.UpdateOrgAddressCommand) error {
 	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
 		org := models.Org{

pkg/services/sqlstore/store.go

@@ -22,7 +22,6 @@ type Store interface {
 	GetOrgByName(name string) (*models.Org, error)
 	CreateOrg(ctx context.Context, cmd *models.CreateOrgCommand) error
 	CreateOrgWithMember(name string, userID int64) (models.Org, error)
-	UpdateOrg(ctx context.Context, cmd *models.UpdateOrgCommand) error
 	UpdateOrgAddress(ctx context.Context, cmd *models.UpdateOrgAddressCommand) error
 	DeleteOrg(ctx context.Context, cmd *models.DeleteOrgCommand) error
 	GetOrgById(context.Context, *models.GetOrgByIdQuery) error