From a7b2943dd1b2ec5059bcce9a42e22b953f35bcc7 Mon Sep 17 00:00:00 2001 From: idafurjes <36131195+idafurjes@users.noreply.github.com> Date: Mon, 28 Nov 2022 14:59:40 +0100 Subject: [PATCH] Chore: Delete SetUsingOrg from sqlstore (#59276) * Chore: Delete SetUsingOrg drom sqlstore * Adjust test * Move test to dashboards * Move test --- .../service/dashboard_service_test.go | 22 +++++++ pkg/services/sqlstore/org_test.go | 58 ------------------- pkg/services/sqlstore/user.go | 22 ------- pkg/services/user/userimpl/store_test.go | 11 ---- pkg/services/user/userimpl/user_test.go | 28 +++++++++ 5 files changed, 50 insertions(+), 91 deletions(-) diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index e2ae5c80bcd..3464677f40e 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -255,4 +255,26 @@ func TestDashboardService(t *testing.T) { err := service.DeleteACLByUser(context.Background(), 1) require.NoError(t, err) }) + + t.Run("When org user is deleted", func(t *testing.T) { + fakeStore := dashboards.FakeDashboardStore{} + fakeStore.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardACLInfoListQuery")).Return(nil) + t.Run("Should remove dependent permissions for deleted org user", func(t *testing.T) { + permQuery := &models.GetDashboardACLInfoListQuery{DashboardID: 1, OrgID: 1, Result: nil} + + err := fakeStore.GetDashboardACLInfoList(context.Background(), permQuery) + require.NoError(t, err) + + require.Equal(t, len(permQuery.Result), 0) + }) + + t.Run("Should not remove dashboard permissions for same user in another org", func(t *testing.T) { + fakeStore := dashboards.FakeDashboardStore{} + fakeStore.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardACLInfoListQuery")).Return(nil) + permQuery := &models.GetDashboardACLInfoListQuery{DashboardID: 2, OrgID: 3} + + err := fakeStore.GetDashboardACLInfoList(context.Background(), permQuery) + require.NoError(t, err) + }) + }) } diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 4a085f55e7d..16099f7a33d 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -100,37 +100,6 @@ func TestIntegrationAccountDataAccess(t *testing.T) { require.Equal(t, len(query.Result), 2) }) - t.Run("Can set using org", func(t *testing.T) { - cmd := models.SetUsingOrgCommand{UserId: ac2.ID, OrgId: ac1.OrgID} - err := sqlStore.SetUsingOrg(context.Background(), &cmd) - require.NoError(t, err) - - t.Run("SignedInUserQuery with a different org", func(t *testing.T) { - query := models.GetSignedInUserQuery{UserId: ac2.ID} - err := sqlStore.GetSignedInUser(context.Background(), &query) - - require.NoError(t, err) - require.Equal(t, query.Result.OrgID, ac1.OrgID) - require.Equal(t, query.Result.Email, "ac2@test.com") - require.Equal(t, query.Result.Name, "ac2 name") - require.Equal(t, query.Result.Login, "ac2") - require.Equal(t, query.Result.OrgName, "ac1@test.com") - }) - - // TODO: This test should be moved to user store - // t.Run("Should set last org as current when removing user from current", func(t *testing.T) { - // remCmd := models.RemoveOrgUserCommand{OrgId: ac1.OrgID, UserId: ac2.ID} - // err := sqlStore.RemoveOrgUser(context.Background(), &remCmd) - // require.NoError(t, err) - - // query := models.GetSignedInUserQuery{UserId: ac2.ID} - // err = sqlStore.GetSignedInUser(context.Background(), &query) - - // require.NoError(t, err) - // require.Equal(t, query.Result.OrgID, ac2.OrgID) - // }) - }) - t.Run("Given an org user with dashboard permissions", func(t *testing.T) { ac3cmd := user.CreateUserCommand{Login: "ac3", Email: "ac3@test.com", Name: "ac3 name", IsAdmin: false} ac3, err := sqlStore.CreateUser(context.Background(), ac3cmd) @@ -157,33 +126,6 @@ func TestIntegrationAccountDataAccess(t *testing.T) { DashboardID: dash2.Id, OrgID: ac3.OrgID, UserID: ac3.ID, Permission: models.PERMISSION_EDIT, }) require.NoError(t, err) - - // TODO: should be moved to dashboard service - // t.Run("When org user is deleted", func(t *testing.T) { - // cmdRemove := models.RemoveOrgUserCommand{OrgId: ac1.OrgID, UserId: ac3.ID} - // err := sqlStore.RemoveOrgUser(context.Background(), &cmdRemove) - // require.NoError(t, err) - - // t.Run("Should remove dependent permissions for deleted org user", func(t *testing.T) { - // permQuery := &models.GetDashboardACLInfoListQuery{DashboardID: dash1.Id, OrgID: ac1.OrgID} - - // err = getDashboardACLInfoList(sqlStore, permQuery) - // require.NoError(t, err) - - // require.Equal(t, len(permQuery.Result), 0) - // }) - - // t.Run("Should not remove dashboard permissions for same user in another org", func(t *testing.T) { - // permQuery := &models.GetDashboardACLInfoListQuery{DashboardID: dash2.Id, OrgID: ac3.OrgID} - - // err = getDashboardACLInfoList(sqlStore, permQuery) - // require.NoError(t, err) - - // require.Equal(t, len(permQuery.Result), 1) - // require.Equal(t, permQuery.Result[0].OrgId, ac3.OrgID) - // require.Equal(t, permQuery.Result[0].UserId, ac3.ID) - // }) - // }) }) }) }) diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 9b97b5fda9c..4064ffc696b 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -160,28 +160,6 @@ func NotServiceAccountFilter(ss *SQLStore) string { ss.Dialect.BooleanStr(false)) } -// deprecated method, use only for tests -func (ss *SQLStore) SetUsingOrg(ctx context.Context, cmd *models.SetUsingOrgCommand) error { - getOrgsForUserCmd := &models.GetUserOrgListQuery{UserId: cmd.UserId} - if err := ss.GetUserOrgList(ctx, getOrgsForUserCmd); err != nil { - return err - } - - valid := false - for _, other := range getOrgsForUserCmd.Result { - if other.OrgId == cmd.OrgId { - valid = true - } - } - if !valid { - return fmt.Errorf("user does not belong to org") - } - - return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { - return setUsingOrgInTransaction(sess, cmd.UserId, cmd.OrgId) - }) -} - func setUsingOrgInTransaction(sess *DBSession, userID int64, orgID int64) error { user := user.User{ ID: userID, diff --git a/pkg/services/user/userimpl/store_test.go b/pkg/services/user/userimpl/store_test.go index ee50e622781..90c1d1a2aa7 100644 --- a/pkg/services/user/userimpl/store_test.go +++ b/pkg/services/user/userimpl/store_test.go @@ -459,17 +459,6 @@ func TestIntegrationUserDataAccess(t *testing.T) { require.Nil(t, err) require.NotNil(t, query3.Result) require.Equal(t, query3.OrgId, users[1].OrgID) - err = ss.SetUsingOrg(context.Background(), &models.SetUsingOrgCommand{UserId: users[1].ID, OrgId: users[0].OrgID}) - require.Nil(t, err) - query4 := &models.GetSignedInUserQuery{OrgId: 0, UserId: users[1].ID} - err = ss.GetSignedInUserWithCacheCtx(context.Background(), query4) - require.Nil(t, err) - require.NotNil(t, query4.Result) - require.Equal(t, query4.Result.OrgID, users[0].OrgID) - - cacheKey := newSignedInUserCacheKey(query4.Result.OrgID, query4.UserId) - _, found := ss.CacheService.Get(cacheKey) - require.True(t, found) disableCmd := user.BatchDisableUsersCommand{ UserIDs: []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID, users[4].ID}, diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index aadd510e2ad..d491bd4f848 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -24,6 +24,7 @@ func TestUserService(t *testing.T) { store: userStore, orgService: orgService, cacheService: localcache.ProvideService(), + teamService: &teamtest.FakeService{}, } t.Run("create user", func(t *testing.T) { @@ -162,6 +163,33 @@ func TestUserService(t *testing.T) { require.Equal(t, roletype.RoleType(userService.cfg.AnonymousOrgRole), u.OrgRole) }) }) + + t.Run("Can set using org", func(t *testing.T) { + cmd := user.SetUsingOrgCommand{UserID: 2, OrgID: 1} + orgService.ExpectedUserOrgDTO = []*org.UserOrgDTO{{OrgID: 1}} + userStore.ExpectedError = nil + err := userService.SetUsingOrg(context.Background(), &cmd) + require.NoError(t, err) + + t.Run("SignedInUserQuery with a different org", func(t *testing.T) { + query := user.GetSignedInUserQuery{UserID: 2} + userStore.ExpectedSignedInUser = &user.SignedInUser{ + OrgID: 1, + Email: "ac2@test.com", + Name: "ac2 name", + Login: "ac2", + OrgName: "ac1@test.com", + } + queryResult, err := userService.GetSignedInUser(context.Background(), &query) + + require.NoError(t, err) + require.EqualValues(t, queryResult.OrgID, 1) + require.Equal(t, queryResult.Email, "ac2@test.com") + require.Equal(t, queryResult.Name, "ac2 name") + require.Equal(t, queryResult.Login, "ac2") + require.Equal(t, queryResult.OrgName, "ac1@test.com") + }) + }) } type FakeUserStore struct {