From 490a787d9d4cb6b43f2edc88e590728eeeeb2717 Mon Sep 17 00:00:00 2001 From: idafurjes <36131195+idafurjes@users.noreply.github.com> Date: Fri, 13 Jan 2023 09:43:38 +0100 Subject: [PATCH] Chore: Move tem member models to team pkg (#61294) * Chore: Move tem member models to team pkg * Fix test lint --- pkg/api/team_members.go | 33 ++-- pkg/api/team_members_test.go | 23 +-- pkg/models/team_member.go | 79 ---------- .../ossaccesscontrol/permissions_services.go | 8 +- .../accesscontrol/team_membership.go | 15 +- .../migrations/accesscontrol/test/ac_test.go | 32 ++-- pkg/services/team/model.go | 67 ++++++++ pkg/services/team/team.go | 8 +- pkg/services/team/teamimpl/store.go | 77 ++++----- pkg/services/team/teamimpl/store_test.go | 149 +++++++++--------- pkg/services/team/teamimpl/team.go | 8 +- pkg/services/team/teamtest/team.go | 12 +- .../teamguardian/database/database.go | 8 +- .../teamguardian/database/database_mock.go | 6 +- pkg/services/teamguardian/manager/service.go | 10 +- .../teamguardian/manager/service_test.go | 18 +-- pkg/services/teamguardian/team.go | 4 +- 17 files changed, 275 insertions(+), 282 deletions(-) delete mode 100644 pkg/models/team_member.go diff --git a/pkg/api/team_members.go b/pkg/api/team_members.go index 111d500ec43..17748226a90 100644 --- a/pkg/api/team_members.go +++ b/pkg/api/team_members.go @@ -33,27 +33,28 @@ func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "teamId is invalid", err) } - query := models.GetTeamMembersQuery{OrgId: c.OrgID, TeamId: teamId, SignedInUser: c.SignedInUser} + query := team.GetTeamMembersQuery{OrgID: c.OrgID, TeamID: teamId, SignedInUser: c.SignedInUser} // With accesscontrol the permission check has been done at middleware layer // and the membership filtering will be done at DB layer based on user permissions if hs.AccessControl.IsDisabled() { - if err := hs.teamGuardian.CanAdmin(c.Req.Context(), query.OrgId, query.TeamId, c.SignedInUser); err != nil { + if err := hs.teamGuardian.CanAdmin(c.Req.Context(), query.OrgID, query.TeamID, c.SignedInUser); err != nil { return response.Error(403, "Not allowed to list team members", err) } } - if err := hs.teamService.GetTeamMembers(c.Req.Context(), &query); err != nil { + queryResult, err := hs.teamService.GetTeamMembers(c.Req.Context(), &query) + if err != nil { return response.Error(500, "Failed to get Team Members", err) } - filteredMembers := make([]*models.TeamMemberDTO, 0, len(query.Result)) - for _, member := range query.Result { + filteredMembers := make([]*team.TeamMemberDTO, 0, len(queryResult)) + for _, member := range queryResult { if dtos.IsHiddenUser(member.Login, c.SignedInUser, hs.Cfg) { continue } - member.AvatarUrl = dtos.GetGravatarUrl(member.Email) + member.AvatarURL = dtos.GetGravatarUrl(member.Email) member.Labels = []string{} if hs.License.FeatureEnabled("teamgroupsync") && member.External { @@ -78,24 +79,24 @@ func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) response.Response { // 404: notFoundError // 500: internalServerError func (hs *HTTPServer) AddTeamMember(c *models.ReqContext) response.Response { - cmd := models.AddTeamMemberCommand{} + cmd := team.AddTeamMemberCommand{} var err error if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } - cmd.OrgId = c.OrgID - cmd.TeamId, err = strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64) + cmd.OrgID = c.OrgID + cmd.TeamID, err = strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64) if err != nil { return response.Error(http.StatusBadRequest, "teamId is invalid", err) } if hs.AccessControl.IsDisabled() { - if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgId, cmd.TeamId, c.SignedInUser); err != nil { + if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgID, cmd.TeamID, c.SignedInUser); err != nil { return response.Error(403, "Not allowed to add team member", err) } } - isTeamMember, err := hs.teamService.IsTeamMember(c.OrgID, cmd.TeamId, cmd.UserId) + isTeamMember, err := hs.teamService.IsTeamMember(c.OrgID, cmd.TeamID, cmd.UserID) if err != nil { return response.Error(500, "Failed to add team member.", err) } @@ -103,7 +104,7 @@ func (hs *HTTPServer) AddTeamMember(c *models.ReqContext) response.Response { return response.Error(400, "User is already added to this team", nil) } - err = addOrUpdateTeamMember(c.Req.Context(), hs.teamPermissionsService, cmd.UserId, cmd.OrgId, cmd.TeamId, getPermissionName(cmd.Permission)) + err = addOrUpdateTeamMember(c.Req.Context(), hs.teamPermissionsService, cmd.UserID, cmd.OrgID, cmd.TeamID, getPermissionName(cmd.Permission)) if err != nil { return response.Error(500, "Failed to add Member to Team", err) } @@ -124,7 +125,7 @@ func (hs *HTTPServer) AddTeamMember(c *models.ReqContext) response.Response { // 404: notFoundError // 500: internalServerError func (hs *HTTPServer) UpdateTeamMember(c *models.ReqContext) response.Response { - cmd := models.UpdateTeamMemberCommand{} + cmd := team.UpdateTeamMemberCommand{} if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } @@ -233,7 +234,7 @@ type GetTeamMembersParams struct { type AddTeamMemberParams struct { // in:body // required:true - Body models.AddTeamMemberCommand `json:"body"` + Body team.AddTeamMemberCommand `json:"body"` // in:path // required:true TeamID string `json:"team_id"` @@ -243,7 +244,7 @@ type AddTeamMemberParams struct { type UpdateTeamMemberParams struct { // in:body // required:true - Body models.UpdateTeamMemberCommand `json:"body"` + Body team.UpdateTeamMemberCommand `json:"body"` // in:path // required:true TeamID string `json:"team_id"` @@ -266,5 +267,5 @@ type RemoveTeamMemberParams struct { type GetTeamMembersResponse struct { // The response message // in: body - Body []*models.TeamMemberDTO `json:"body"` + Body []*team.TeamMemberDTO `json:"body"` } diff --git a/pkg/api/team_members_test.go b/pkg/api/team_members_test.go index a1517189aba..ead50489d7e 100644 --- a/pkg/api/team_members_test.go +++ b/pkg/api/team_members_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" + "github.com/grafana/grafana/pkg/services/team" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -91,7 +92,7 @@ func TestTeamMembersAPIEndpoint_userLoggedIn(t *testing.T) { require.Equal(t, http.StatusOK, sc.resp.Code) - var resp []models.TeamMemberDTO + var resp []team.TeamMemberDTO err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) require.NoError(t, err) assert.Len(t, resp, 3) @@ -113,7 +114,7 @@ func TestTeamMembersAPIEndpoint_userLoggedIn(t *testing.T) { require.Equal(t, http.StatusOK, sc.resp.Code) - var resp []models.TeamMemberDTO + var resp []team.TeamMemberDTO err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) require.NoError(t, err) assert.Len(t, resp, 3) @@ -132,9 +133,9 @@ func TestAddTeamMembersAPIEndpoint_LegacyAccessControl(t *testing.T) { hs.Cfg = cfg hs.teamService = teamtest.NewFakeService() store := &database.TeamGuardianStoreMock{} - store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*models.TeamMemberDTO{ - {UserId: 2, Permission: models.PERMISSION_ADMIN}, - {UserId: 3, Permission: models.PERMISSION_VIEW}, + store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*team.TeamMemberDTO{ + {UserID: 2, Permission: models.PERMISSION_ADMIN}, + {UserID: 3, Permission: models.PERMISSION_VIEW}, }, nil).Maybe() hs.teamGuardian = manager.ProvideService(store) hs.teamPermissionsService = &actest.FakePermissionsService{} @@ -252,9 +253,9 @@ func TestUpdateTeamMembersAPIEndpoint_LegacyAccessControl(t *testing.T) { hs.Cfg = cfg hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} store := &database.TeamGuardianStoreMock{} - store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*models.TeamMemberDTO{ - {UserId: 2, Permission: models.PERMISSION_ADMIN}, - {UserId: 3, Permission: models.PERMISSION_VIEW}, + store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*team.TeamMemberDTO{ + {UserID: 2, Permission: models.PERMISSION_ADMIN}, + {UserID: 3, Permission: models.PERMISSION_VIEW}, }, nil).Maybe() hs.teamGuardian = manager.ProvideService(store) hs.teamPermissionsService = &actest.FakePermissionsService{} @@ -342,9 +343,9 @@ func TestDeleteTeamMembersAPIEndpoint_LegacyAccessControl(t *testing.T) { hs.Cfg = cfg hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} store := &database.TeamGuardianStoreMock{} - store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*models.TeamMemberDTO{ - {UserId: 2, Permission: models.PERMISSION_ADMIN}, - {UserId: 3, Permission: models.PERMISSION_VIEW}, + store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*team.TeamMemberDTO{ + {UserID: 2, Permission: models.PERMISSION_ADMIN}, + {UserID: 3, Permission: models.PERMISSION_VIEW}, }, nil).Maybe() hs.teamGuardian = manager.ProvideService(store) hs.teamPermissionsService = &actest.FakePermissionsService{} diff --git a/pkg/models/team_member.go b/pkg/models/team_member.go deleted file mode 100644 index 1aea6c5fcdf..00000000000 --- a/pkg/models/team_member.go +++ /dev/null @@ -1,79 +0,0 @@ -package models - -import ( - "errors" - "time" - - "github.com/grafana/grafana/pkg/services/user" -) - -// Typed errors -var ( - ErrTeamMemberAlreadyAdded = errors.New("user is already added to this team") -) - -// TeamMember model -type TeamMember struct { - Id int64 - OrgId int64 - TeamId int64 - UserId int64 - External bool // Signals that the membership has been created by an external systems, such as LDAP - Permission PermissionType - - Created time.Time - Updated time.Time -} - -// --------------------- -// COMMANDS - -type AddTeamMemberCommand struct { - UserId int64 `json:"userId" binding:"Required"` - OrgId int64 `json:"-"` - TeamId int64 `json:"-"` - External bool `json:"-"` - Permission PermissionType `json:"-"` -} - -type UpdateTeamMemberCommand struct { - UserId int64 `json:"-"` - OrgId int64 `json:"-"` - TeamId int64 `json:"-"` - Permission PermissionType `json:"permission"` -} - -type RemoveTeamMemberCommand struct { - OrgId int64 `json:"-"` - UserId int64 - TeamId int64 -} - -// ---------------------- -// QUERIES - -type GetTeamMembersQuery struct { - OrgId int64 - TeamId int64 - UserId int64 - External bool - SignedInUser *user.SignedInUser - Result []*TeamMemberDTO -} - -// ---------------------- -// Projections and DTOs - -type TeamMemberDTO struct { - OrgId int64 `json:"orgId"` - TeamId int64 `json:"teamId"` - UserId int64 `json:"userId"` - External bool `json:"-"` - AuthModule string `json:"auth_module"` - Email string `json:"email"` - Name string `json:"name"` - Login string `json:"login"` - AvatarUrl string `json:"avatarUrl"` - Labels []string `json:"labels"` - Permission PermissionType `json:"permission"` -} diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index 7c9365e8cd1..476477980ee 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -86,10 +86,10 @@ func ProvideTeamPermissions( case "Admin": return teamimpl.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, models.PERMISSION_ADMIN) case "": - return teamimpl.RemoveTeamMemberHook(session, &models.RemoveTeamMemberCommand{ - OrgId: orgID, - UserId: user.ID, - TeamId: teamId, + return teamimpl.RemoveTeamMemberHook(session, &team.RemoveTeamMemberCommand{ + OrgID: orgID, + UserID: user.ID, + TeamID: teamId, }) default: return fmt.Errorf("invalid team permission type %s", permission) diff --git a/pkg/services/sqlstore/migrations/accesscontrol/team_membership.go b/pkg/services/sqlstore/migrations/accesscontrol/team_membership.go index d04b182b622..a0be96f92a9 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/team_membership.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/team_membership.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" + "github.com/grafana/grafana/pkg/services/team" ) const ( @@ -112,7 +113,7 @@ func (p *teamPermissionMigrator) migrateMemberships() error { } // Fetch team memberships - teamMemberships := []*models.TeamMember{} + teamMemberships := []*team.TeamMember{} if err := p.sess.SQL("SELECT * FROM team_member").Find(&teamMemberships); err != nil { return err } @@ -201,7 +202,7 @@ func (p *teamPermissionMigrator) sortRolesToAssign(userPermissionsByOrg map[int6 return rolesToCreate, rolesByOrg, nil } -func (p *teamPermissionMigrator) generateAssociatedPermissions(teamMemberships []*models.TeamMember, +func (p *teamPermissionMigrator) generateAssociatedPermissions(teamMemberships []*team.TeamMember, userRolesByOrg map[int64]map[int64]string) (map[int64]map[int64][]accesscontrol.Permission, error) { userPermissionsByOrg := map[int64]map[int64][]accesscontrol.Permission{} @@ -210,21 +211,21 @@ func (p *teamPermissionMigrator) generateAssociatedPermissions(teamMemberships [ // only admins or editors (when editorsCanAdmin option is enabled) // can access team administration endpoints if m.Permission == models.PERMISSION_ADMIN { - if userRolesByOrg[m.OrgId][m.UserId] == string(org.RoleViewer) || (userRolesByOrg[m.OrgId][m.UserId] == string(org.RoleEditor) && !p.editorsCanAdmin) { + if userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleViewer) || (userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleEditor) && !p.editorsCanAdmin) { m.Permission = 0 - if _, err := p.sess.Cols("permission").Where("org_id=? and team_id=? and user_id=?", m.OrgId, m.TeamId, m.UserId).Update(m); err != nil { + if _, err := p.sess.Cols("permission").Where("org_id=? and team_id=? and user_id=?", m.OrgID, m.TeamID, m.UserID).Update(m); err != nil { return nil, err } } } - userPermissions, initialized := userPermissionsByOrg[m.OrgId] + userPermissions, initialized := userPermissionsByOrg[m.OrgID] if !initialized { userPermissions = map[int64][]accesscontrol.Permission{} } - userPermissions[m.UserId] = append(userPermissions[m.UserId], p.mapPermissionToRBAC(m.Permission, m.TeamId)...) - userPermissionsByOrg[m.OrgId] = userPermissions + userPermissions[m.UserID] = append(userPermissions[m.UserID], p.mapPermissionToRBAC(m.Permission, m.TeamID)...) + userPermissionsByOrg[m.OrgID] = userPermissions } return userPermissionsByOrg, nil diff --git a/pkg/services/sqlstore/migrations/accesscontrol/test/ac_test.go b/pkg/services/sqlstore/migrations/accesscontrol/test/ac_test.go index 9aee3a61cb9..a1fb2e3d72a 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/test/ac_test.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/test/ac_test.go @@ -336,12 +336,12 @@ func setupTeams(t *testing.T, x *xorm.Engine) { require.NoError(t, errInsertTeams) require.Equal(t, int64(2), teamCount, "needed 2 teams for this test to run") - members := []models.TeamMember{ + members := []team.TeamMember{ { // Can have viewer permissions - OrgId: 1, - TeamId: 1, - UserId: 1, + OrgID: 1, + TeamID: 1, + UserID: 1, External: false, Permission: 0, Created: now, @@ -349,9 +349,9 @@ func setupTeams(t *testing.T, x *xorm.Engine) { }, { // Cannot have admin permissions - OrgId: 1, - TeamId: 1, - UserId: 2, + OrgID: 1, + TeamID: 1, + UserID: 2, External: false, Permission: models.PERMISSION_ADMIN, Created: now, @@ -359,9 +359,9 @@ func setupTeams(t *testing.T, x *xorm.Engine) { }, { // Can have admin permissions - OrgId: 1, - TeamId: 1, - UserId: 3, + OrgID: 1, + TeamID: 1, + UserID: 3, External: false, Permission: models.PERMISSION_ADMIN, Created: now, @@ -369,9 +369,9 @@ func setupTeams(t *testing.T, x *xorm.Engine) { }, { // Can have admin permissions - OrgId: 1, - TeamId: 1, - UserId: 4, + OrgID: 1, + TeamID: 1, + UserID: 4, External: false, Permission: models.PERMISSION_ADMIN, Created: now, @@ -379,9 +379,9 @@ func setupTeams(t *testing.T, x *xorm.Engine) { }, { // Can have viewer permissions - OrgId: 2, - TeamId: 2, - UserId: 5, + OrgID: 2, + TeamID: 2, + UserID: 5, External: false, Permission: 0, Created: now, diff --git a/pkg/services/team/model.go b/pkg/services/team/model.go index efc91710bba..f7d3b347d74 100644 --- a/pkg/services/team/model.go +++ b/pkg/services/team/model.go @@ -16,6 +16,8 @@ var ( ErrLastTeamAdmin = errors.New("not allowed to remove last admin") ErrNotAllowedToUpdateTeam = errors.New("user not allowed to update team") ErrNotAllowedToUpdateTeamInDifferentOrg = errors.New("user not allowed to update team in another org") + + ErrTeamMemberAlreadyAdded = errors.New("user is already added to this team") ) // Team model @@ -99,3 +101,68 @@ type SearchTeamQueryResult struct { type IsAdminOfTeamsQuery struct { SignedInUser *user.SignedInUser } + +// TeamMember model +type TeamMember struct { + ID int64 `xorm:"pk autoincr 'id'"` + OrgID int64 `xorm:"org_id"` + TeamID int64 `xorm:"team_id"` + UserID int64 `xorm:"user_id"` + External bool // Signals that the membership has been created by an external systems, such as LDAP + Permission models.PermissionType + + Created time.Time + Updated time.Time +} + +// --------------------- +// COMMANDS + +type AddTeamMemberCommand struct { + UserID int64 `json:"userId" binding:"Required"` + OrgID int64 `json:"-"` + TeamID int64 `json:"-"` + External bool `json:"-"` + Permission models.PermissionType `json:"-"` +} + +type UpdateTeamMemberCommand struct { + UserID int64 `json:"-"` + OrgID int64 `json:"-"` + TeamID int64 `json:"-"` + Permission models.PermissionType `json:"permission"` +} + +type RemoveTeamMemberCommand struct { + OrgID int64 `json:"-"` + UserID int64 + TeamID int64 +} + +// ---------------------- +// QUERIES + +type GetTeamMembersQuery struct { + OrgID int64 + TeamID int64 + UserID int64 + External bool + SignedInUser *user.SignedInUser +} + +// ---------------------- +// Projections and DTOs + +type TeamMemberDTO struct { + OrgID int64 `json:"orgId" xorm:"org_id"` + TeamID int64 `json:"teamId" xorm:"team_id"` + UserID int64 `json:"userId" xorm:"user_id"` + External bool `json:"-"` + AuthModule string `json:"auth_module"` + Email string `json:"email"` + Name string `json:"name"` + Login string `json:"login"` + AvatarURL string `json:"avatarUrl" xorm:"avatar_url"` + Labels []string `json:"labels"` + Permission models.PermissionType `json:"permission"` +} diff --git a/pkg/services/team/team.go b/pkg/services/team/team.go index aff81ee0066..730a5cc0aa0 100644 --- a/pkg/services/team/team.go +++ b/pkg/services/team/team.go @@ -14,10 +14,10 @@ type Service interface { GetTeamByID(ctx context.Context, query *GetTeamByIDQuery) (*TeamDTO, error) GetTeamsByUser(ctx context.Context, query *GetTeamsByUserQuery) ([]*TeamDTO, error) AddTeamMember(userID, orgID, teamID int64, isExternal bool, permission models.PermissionType) error - UpdateTeamMember(ctx context.Context, cmd *models.UpdateTeamMemberCommand) error + UpdateTeamMember(ctx context.Context, cmd *UpdateTeamMemberCommand) error IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error) - RemoveTeamMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error - GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error) - GetTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery) error + RemoveTeamMember(ctx context.Context, cmd *RemoveTeamMemberCommand) error + GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*TeamMemberDTO, error) + GetTeamMembers(ctx context.Context, query *GetTeamMembersQuery) ([]*TeamMemberDTO, error) IsAdminOfTeams(ctx context.Context, query *IsAdminOfTeamsQuery) (bool, error) } diff --git a/pkg/services/team/teamimpl/store.go b/pkg/services/team/teamimpl/store.go index 97f169a595a..c5eadcc36db 100644 --- a/pkg/services/team/teamimpl/store.go +++ b/pkg/services/team/teamimpl/store.go @@ -23,11 +23,11 @@ type store interface { GetByID(ctx context.Context, query *team.GetTeamByIDQuery) (*team.TeamDTO, error) GetByUser(ctx context.Context, query *team.GetTeamsByUserQuery) ([]*team.TeamDTO, error) AddMember(userID, orgID, teamID int64, isExternal bool, permission models.PermissionType) error - UpdateMember(ctx context.Context, cmd *models.UpdateTeamMemberCommand) error + UpdateMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error IsMember(orgId int64, teamId int64, userId int64) (bool, error) - RemoveMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error - GetMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error) - GetMembers(ctx context.Context, query *models.GetTeamMembersQuery) error + RemoveMember(ctx context.Context, cmd *team.RemoveTeamMemberCommand) error + GetMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*team.TeamMemberDTO, error) + GetMembers(ctx context.Context, query *team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) IsAdmin(ctx context.Context, query *team.IsAdminOfTeamsQuery) (bool, error) } @@ -363,16 +363,16 @@ func (ss *xormStore) AddMember(userID, orgID, teamID int64, isExternal bool, per if isMember, err := isTeamMember(sess, orgID, teamID, userID); err != nil { return err } else if isMember { - return models.ErrTeamMemberAlreadyAdded + return team.ErrTeamMemberAlreadyAdded } return addTeamMember(sess, orgID, teamID, userID, isExternal, permission) }) } -func getTeamMember(sess *db.Session, orgId int64, teamId int64, userId int64) (models.TeamMember, error) { +func getTeamMember(sess *db.Session, orgId int64, teamId int64, userId int64) (team.TeamMember, error) { rawSQL := `SELECT * FROM team_member WHERE org_id=? and team_id=? and user_id=?` - var member models.TeamMember + var member team.TeamMember exists, err := sess.SQL(rawSQL, orgId, teamId, userId).Get(&member) if err != nil { @@ -386,9 +386,9 @@ func getTeamMember(sess *db.Session, orgId int64, teamId int64, userId int64) (m } // UpdateTeamMember updates a team member -func (ss *xormStore) UpdateMember(ctx context.Context, cmd *models.UpdateTeamMemberCommand) error { +func (ss *xormStore) UpdateMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error { return ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { - return updateTeamMember(sess, cmd.OrgId, cmd.TeamId, cmd.UserId, cmd.Permission) + return updateTeamMember(sess, cmd.OrgID, cmd.TeamID, cmd.UserID, cmd.Permission) }) } @@ -436,10 +436,10 @@ func addTeamMember(sess *db.Session, orgID, teamID, userID int64, isExternal boo return err } - entity := models.TeamMember{ - OrgId: orgID, - TeamId: teamID, - UserId: userID, + entity := team.TeamMember{ + OrgID: orgID, + TeamID: teamID, + UserID: userID, External: isExternal, Created: time.Now(), Updated: time.Now(), @@ -466,7 +466,7 @@ func updateTeamMember(sess *db.Session, orgID, teamID, userID int64, permission } // RemoveTeamMember removes a member from a team -func (ss *xormStore) RemoveMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error { +func (ss *xormStore) RemoveMember(ctx context.Context, cmd *team.RemoveTeamMemberCommand) error { return ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { return removeTeamMember(sess, cmd) }) @@ -474,17 +474,17 @@ func (ss *xormStore) RemoveMember(ctx context.Context, cmd *models.RemoveTeamMem // RemoveTeamMemberHook is called from team resource permission service // it removes a member from a team within the given transaction session -func RemoveTeamMemberHook(sess *db.Session, cmd *models.RemoveTeamMemberCommand) error { +func RemoveTeamMemberHook(sess *db.Session, cmd *team.RemoveTeamMemberCommand) error { return removeTeamMember(sess, cmd) } -func removeTeamMember(sess *db.Session, cmd *models.RemoveTeamMemberCommand) error { - if _, err := teamExists(cmd.OrgId, cmd.TeamId, sess); err != nil { +func removeTeamMember(sess *db.Session, cmd *team.RemoveTeamMemberCommand) error { + if _, err := teamExists(cmd.OrgID, cmd.TeamID, sess); err != nil { return err } var rawSQL = "DELETE FROM team_member WHERE org_id=? and team_id=? and user_id=?" - res, err := sess.Exec(rawSQL, cmd.OrgId, cmd.TeamId, cmd.UserId) + res, err := sess.Exec(rawSQL, cmd.OrgID, cmd.TeamID, cmd.UserID) if err != nil { return err } @@ -499,19 +499,18 @@ func removeTeamMember(sess *db.Session, cmd *models.RemoveTeamMemberCommand) err // GetUserTeamMemberships return a list of memberships to teams granted to a user // If external is specified, only memberships provided by an external auth provider will be listed // This function doesn't perform any accesscontrol filtering. -func (ss *xormStore) GetMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error) { - query := &models.GetTeamMembersQuery{ - OrgId: orgID, - UserId: userID, +func (ss *xormStore) GetMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*team.TeamMemberDTO, error) { + query := &team.GetTeamMembersQuery{ + OrgID: orgID, + UserID: userID, External: external, - Result: []*models.TeamMemberDTO{}, } - err := ss.getTeamMembers(ctx, query, nil) - return query.Result, err + queryResult, err := ss.getTeamMembers(ctx, query, nil) + return queryResult, err } // GetTeamMembers return a list of members for the specified team filtered based on the user's permissions -func (ss *xormStore) GetMembers(ctx context.Context, query *models.GetTeamMembersQuery) error { +func (ss *xormStore) GetMembers(ctx context.Context, query *team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) { acFilter := &ac.SQLFilter{} var err error @@ -522,7 +521,7 @@ func (ss *xormStore) GetMembers(ctx context.Context, query *models.GetTeamMember sqlID := fmt.Sprintf("%s.%s", ss.db.GetDialect().Quote("user"), ss.db.GetDialect().Quote("id")) *acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users:id:", ac.ActionOrgUsersRead) if err != nil { - return err + return nil, err } } @@ -530,9 +529,9 @@ func (ss *xormStore) GetMembers(ctx context.Context, query *models.GetTeamMember } // getTeamMembers return a list of members for the specified team -func (ss *xormStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acUserFilter *ac.SQLFilter) error { - return ss.db.WithDbSession(ctx, func(dbSess *db.Session) error { - query.Result = make([]*models.TeamMemberDTO, 0) +func (ss *xormStore) getTeamMembers(ctx context.Context, query *team.GetTeamMembersQuery, acUserFilter *ac.SQLFilter) ([]*team.TeamMemberDTO, error) { + queryResult := make([]*team.TeamMemberDTO, 0) + err := ss.db.WithDbSession(ctx, func(dbSess *db.Session) error { sess := dbSess.Table("team_member") sess.Join("INNER", ss.db.GetDialect().Quote("user"), fmt.Sprintf("team_member.user_id=%s.%s", ss.db.GetDialect().Quote("user"), ss.db.GetDialect().Quote("id")), @@ -553,14 +552,14 @@ func (ss *xormStore) getTeamMembers(ctx context.Context, query *models.GetTeamMe authJoinCondition = "user_auth.id=" + authJoinCondition + ss.db.GetDialect().Limit(1) + ")" sess.Join("LEFT", "user_auth", authJoinCondition) - if query.OrgId != 0 { - sess.Where("team_member.org_id=?", query.OrgId) + if query.OrgID != 0 { + sess.Where("team_member.org_id=?", query.OrgID) } - if query.TeamId != 0 { - sess.Where("team_member.team_id=?", query.TeamId) + if query.TeamID != 0 { + sess.Where("team_member.team_id=?", query.TeamID) } - if query.UserId != 0 { - sess.Where("team_member.user_id=?", query.UserId) + if query.UserID != 0 { + sess.Where("team_member.user_id=?", query.UserID) } if query.External { sess.Where("team_member.external=?", ss.db.GetDialect().BooleanStr(true)) @@ -578,9 +577,13 @@ func (ss *xormStore) getTeamMembers(ctx context.Context, query *models.GetTeamMe ) sess.Asc("user.login", "user.email") - err := sess.Find(&query.Result) + err := sess.Find(&queryResult) return err }) + if err != nil { + return nil, err + } + return queryResult, nil } func (ss *xormStore) IsAdmin(ctx context.Context, query *team.IsAdminOfTeamsQuery) (bool, error) { diff --git a/pkg/services/team/teamimpl/store_test.go b/pkg/services/team/teamimpl/store_test.go index 0b783bde5de..5dae4cb200c 100644 --- a/pkg/services/team/teamimpl/store_test.go +++ b/pkg/services/team/teamimpl/store_test.go @@ -88,26 +88,26 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { err = teamSvc.AddTeamMember(userIds[1], testOrgID, team1.ID, true, 0) require.NoError(t, err) - q1 := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team1.ID, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), q1) + q1 := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, SignedInUser: testUser} + q1Result, err := teamSvc.GetTeamMembers(context.Background(), q1) require.NoError(t, err) - require.Equal(t, 2, len(q1.Result)) - require.Equal(t, q1.Result[0].TeamId, team1.ID) - require.Equal(t, q1.Result[0].Login, "loginuser0") - require.Equal(t, q1.Result[0].OrgId, testOrgID) - require.Equal(t, q1.Result[1].TeamId, team1.ID) - require.Equal(t, q1.Result[1].Login, "loginuser1") - require.Equal(t, q1.Result[1].OrgId, testOrgID) - require.Equal(t, q1.Result[1].External, true) + require.Equal(t, 2, len(q1Result)) + require.Equal(t, q1Result[0].TeamID, team1.ID) + require.Equal(t, q1Result[0].Login, "loginuser0") + require.Equal(t, q1Result[0].OrgID, testOrgID) + require.Equal(t, q1Result[1].TeamID, team1.ID) + require.Equal(t, q1Result[1].Login, "loginuser1") + require.Equal(t, q1Result[1].OrgID, testOrgID) + require.Equal(t, q1Result[1].External, true) - q2 := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team1.ID, External: true, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), q2) + q2 := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, External: true, SignedInUser: testUser} + q2Result, err := teamSvc.GetTeamMembers(context.Background(), q2) require.NoError(t, err) - require.Equal(t, len(q2.Result), 1) - require.Equal(t, q2.Result[0].TeamId, team1.ID) - require.Equal(t, q2.Result[0].Login, "loginuser1") - require.Equal(t, q2.Result[0].OrgId, testOrgID) - require.Equal(t, q2.Result[0].External, true) + require.Equal(t, len(q2Result), 1) + require.Equal(t, q2Result[0].TeamID, team1.ID) + require.Equal(t, q2Result[0].Login, "loginuser1") + require.Equal(t, q2Result[0].OrgID, testOrgID) + require.Equal(t, q2Result[0].External, true) queryResult, err = teamSvc.SearchTeams(context.Background(), query) require.NoError(t, err) @@ -139,78 +139,77 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { err = teamSvc.AddTeamMember(userId, testOrgID, team1.ID, true, 0) require.NoError(t, err) - memberQuery := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team1.ID, External: true, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), memberQuery) + memberQuery := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, External: true, SignedInUser: testUser} + memberQueryResult, err := teamSvc.GetTeamMembers(context.Background(), memberQuery) require.NoError(t, err) - require.Equal(t, len(memberQuery.Result), 1) - require.Equal(t, memberQuery.Result[0].TeamId, team1.ID) - require.Equal(t, memberQuery.Result[0].Login, "loginuser1") - require.Equal(t, memberQuery.Result[0].OrgId, testOrgID) - require.Equal(t, memberQuery.Result[0].External, true) + require.Equal(t, len(memberQueryResult), 1) + require.Equal(t, memberQueryResult[0].TeamID, team1.ID) + require.Equal(t, memberQueryResult[0].Login, "loginuser1") + require.Equal(t, memberQueryResult[0].OrgID, testOrgID) + require.Equal(t, memberQueryResult[0].External, true) }) t.Run("Should be able to update users in a team", func(t *testing.T) { userId := userIds[0] - team := team1 - err = teamSvc.AddTeamMember(userId, testOrgID, team.ID, false, 0) + + err = teamSvc.AddTeamMember(userId, testOrgID, team1.ID, false, 0) require.NoError(t, err) - qBeforeUpdate := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team.ID, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), qBeforeUpdate) + qBeforeUpdate := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, SignedInUser: testUser} + qBeforeUpdateResult, err := teamSvc.GetTeamMembers(context.Background(), qBeforeUpdate) require.NoError(t, err) - require.EqualValues(t, qBeforeUpdate.Result[0].Permission, 0) + require.EqualValues(t, qBeforeUpdateResult[0].Permission, 0) - err = teamSvc.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{ - UserId: userId, - OrgId: testOrgID, - TeamId: team.ID, + err = teamSvc.UpdateTeamMember(context.Background(), &team.UpdateTeamMemberCommand{ + UserID: userId, + OrgID: testOrgID, + TeamID: team1.ID, Permission: models.PERMISSION_ADMIN, }) require.NoError(t, err) - qAfterUpdate := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team.ID, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), qAfterUpdate) + qAfterUpdate := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, SignedInUser: testUser} + qAfterUpdateResult, err := teamSvc.GetTeamMembers(context.Background(), qAfterUpdate) require.NoError(t, err) - require.Equal(t, qAfterUpdate.Result[0].Permission, models.PERMISSION_ADMIN) + require.Equal(t, qAfterUpdateResult[0].Permission, models.PERMISSION_ADMIN) }) t.Run("Should default to member permission level when updating a user with invalid permission level", func(t *testing.T) { sqlStore = db.InitTestDB(t) setup() userID := userIds[0] - team := team1 - err = teamSvc.AddTeamMember(userID, testOrgID, team.ID, false, 0) + err = teamSvc.AddTeamMember(userID, testOrgID, team1.ID, false, 0) require.NoError(t, err) - qBeforeUpdate := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team.ID, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), qBeforeUpdate) + qBeforeUpdate := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, SignedInUser: testUser} + qBeforeUpdateResult, err := teamSvc.GetTeamMembers(context.Background(), qBeforeUpdate) require.NoError(t, err) - require.EqualValues(t, qBeforeUpdate.Result[0].Permission, 0) + require.EqualValues(t, qBeforeUpdateResult[0].Permission, 0) invalidPermissionLevel := models.PERMISSION_EDIT - err = teamSvc.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{ - UserId: userID, - OrgId: testOrgID, - TeamId: team.ID, + err = teamSvc.UpdateTeamMember(context.Background(), &team.UpdateTeamMemberCommand{ + UserID: userID, + OrgID: testOrgID, + TeamID: team1.ID, Permission: invalidPermissionLevel, }) require.NoError(t, err) - qAfterUpdate := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team.ID, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), qAfterUpdate) + qAfterUpdate := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, SignedInUser: testUser} + qAfterUpdateResult, err := teamSvc.GetTeamMembers(context.Background(), qAfterUpdate) require.NoError(t, err) - require.EqualValues(t, qAfterUpdate.Result[0].Permission, 0) + require.EqualValues(t, qAfterUpdateResult[0].Permission, 0) }) t.Run("Shouldn't be able to update a user not in the team.", func(t *testing.T) { sqlStore = db.InitTestDB(t) setup() - err = teamSvc.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{ - UserId: 1, - OrgId: testOrgID, - TeamId: team1.ID, + err = teamSvc.UpdateTeamMember(context.Background(), &team.UpdateTeamMemberCommand{ + UserID: 1, + OrgID: testOrgID, + TeamID: team1.ID, Permission: models.PERMISSION_ADMIN, }) @@ -257,13 +256,13 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { err = teamSvc.AddTeamMember(userIds[0], testOrgID, team1.ID, false, 0) require.NoError(t, err) - err = teamSvc.RemoveTeamMember(context.Background(), &models.RemoveTeamMemberCommand{OrgId: testOrgID, TeamId: team1.ID, UserId: userIds[0]}) + err = teamSvc.RemoveTeamMember(context.Background(), &team.RemoveTeamMemberCommand{OrgID: testOrgID, TeamID: team1.ID, UserID: userIds[0]}) require.NoError(t, err) - q2 := &models.GetTeamMembersQuery{OrgId: testOrgID, TeamId: team1.ID, SignedInUser: testUser} - err = teamSvc.GetTeamMembers(context.Background(), q2) + q2 := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, SignedInUser: testUser} + q2Result, err := teamSvc.GetTeamMembers(context.Background(), q2) require.NoError(t, err) - require.Equal(t, len(q2.Result), 0) + require.Equal(t, len(q2Result), 0) }) t.Run("Should have empty teams", func(t *testing.T) { @@ -271,12 +270,12 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { require.NoError(t, err) t.Run("A user should be able to remove the admin permission for the last admin", func(t *testing.T) { - err = teamSvc.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{OrgId: testOrgID, TeamId: team1.ID, UserId: userIds[0], Permission: 0}) + err = teamSvc.UpdateTeamMember(context.Background(), &team.UpdateTeamMemberCommand{OrgID: testOrgID, TeamID: team1.ID, UserID: userIds[0], Permission: 0}) require.NoError(t, err) }) t.Run("A user should be able to remove the last member", func(t *testing.T) { - err = teamSvc.RemoveTeamMember(context.Background(), &models.RemoveTeamMemberCommand{OrgId: testOrgID, TeamId: team1.ID, UserId: userIds[0]}) + err = teamSvc.RemoveTeamMember(context.Background(), &team.RemoveTeamMemberCommand{OrgID: testOrgID, TeamID: team1.ID, UserID: userIds[0]}) require.NoError(t, err) }) @@ -289,7 +288,7 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { err = teamSvc.AddTeamMember(userIds[1], testOrgID, team1.ID, false, models.PERMISSION_ADMIN) require.NoError(t, err) - err = teamSvc.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{OrgId: testOrgID, TeamId: team1.ID, UserId: userIds[0], Permission: 0}) + err = teamSvc.UpdateTeamMember(context.Background(), &team.UpdateTeamMemberCommand{OrgID: testOrgID, TeamID: team1.ID, UserID: userIds[0], Permission: 0}) require.NoError(t, err) }) }) @@ -407,15 +406,15 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { err = teamSvc.AddTeamMember(userIds[0], testOrgID, groupId, false, 0) require.NoError(t, err) - teamMembersQuery := &models.GetTeamMembersQuery{ - OrgId: testOrgID, + teamMembersQuery := &team.GetTeamMembersQuery{ + OrgID: testOrgID, SignedInUser: testUser, - TeamId: groupId, + TeamID: groupId, } - err = teamSvc.GetTeamMembers(context.Background(), teamMembersQuery) + teamMembersQueryResult, err := teamSvc.GetTeamMembers(context.Background(), teamMembersQuery) require.NoError(t, err) // should not receive service account from query - require.Equal(t, len(teamMembersQuery.Result), 1) + require.Equal(t, len(teamMembersQueryResult), 1) }) }) }) @@ -545,15 +544,15 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { type getTeamMembersTestCase struct { desc string - query *models.GetTeamMembersQuery + query *team.GetTeamMembersQuery expectedNumUsers int } tests := []getTeamMembersTestCase{ { desc: "should return all team members", - query: &models.GetTeamMembersQuery{ - OrgId: testOrgID, + query: &team.GetTeamMembersQuery{ + OrgID: testOrgID, SignedInUser: &user.SignedInUser{ OrgID: testOrgID, Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}}}, @@ -563,8 +562,8 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { }, { desc: "should return no team members", - query: &models.GetTeamMembersQuery{ - OrgId: testOrgID, + query: &team.GetTeamMembersQuery{ + OrgID: testOrgID, SignedInUser: &user.SignedInUser{ OrgID: testOrgID, Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {""}}}, @@ -575,8 +574,8 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { { desc: "should return some team members", - query: &models.GetTeamMembersQuery{ - OrgId: testOrgID, + query: &team.GetTeamMembersQuery{ + OrgID: testOrgID, SignedInUser: &user.SignedInUser{ OrgID: testOrgID, Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: { @@ -591,15 +590,15 @@ func TestIntegrationSQLStore_GetTeamMembers_ACFilter(t *testing.T) { } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - err := teamSvc.GetTeamMembers(context.Background(), tt.query) + queryResult, err := teamSvc.GetTeamMembers(context.Background(), tt.query) require.NoError(t, err) - assert.Len(t, tt.query.Result, tt.expectedNumUsers) + assert.Len(t, queryResult, tt.expectedNumUsers) if !hasWildcardScope(tt.query.SignedInUser, ac.ActionOrgUsersRead) { - for _, member := range tt.query.Result { + for _, member := range queryResult { assert.Contains(t, tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgID][ac.ActionOrgUsersRead], - ac.Scope("users", "id", fmt.Sprintf("%d", member.UserId)), + ac.Scope("users", "id", fmt.Sprintf("%d", member.UserID)), ) } } diff --git a/pkg/services/team/teamimpl/team.go b/pkg/services/team/teamimpl/team.go index e18ee3cc5ba..f9a33d0715f 100644 --- a/pkg/services/team/teamimpl/team.go +++ b/pkg/services/team/teamimpl/team.go @@ -45,7 +45,7 @@ func (s *Service) AddTeamMember(userID, orgID, teamID int64, isExternal bool, pe return s.store.AddMember(userID, orgID, teamID, isExternal, permission) } -func (s *Service) UpdateTeamMember(ctx context.Context, cmd *models.UpdateTeamMemberCommand) error { +func (s *Service) UpdateTeamMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error { return s.store.UpdateMember(ctx, cmd) } @@ -53,15 +53,15 @@ func (s *Service) IsTeamMember(orgId int64, teamId int64, userId int64) (bool, e return s.store.IsMember(orgId, teamId, userId) } -func (s *Service) RemoveTeamMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error { +func (s *Service) RemoveTeamMember(ctx context.Context, cmd *team.RemoveTeamMemberCommand) error { return s.store.RemoveMember(ctx, cmd) } -func (s *Service) GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error) { +func (s *Service) GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*team.TeamMemberDTO, error) { return s.store.GetMemberships(ctx, orgID, userID, external) } -func (s *Service) GetTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery) error { +func (s *Service) GetTeamMembers(ctx context.Context, query *team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) { return s.store.GetMembers(ctx, query) } diff --git a/pkg/services/team/teamtest/team.go b/pkg/services/team/teamtest/team.go index 7d02ac858e9..5baf567c5e1 100644 --- a/pkg/services/team/teamtest/team.go +++ b/pkg/services/team/teamtest/team.go @@ -12,7 +12,7 @@ type FakeService struct { ExpectedIsMember bool ExpectedTeamDTO *team.TeamDTO ExpectedTeamsByUser []*team.TeamDTO - ExpectedMembers []*models.TeamMemberDTO + ExpectedMembers []*team.TeamMemberDTO ExpectedError error } @@ -48,7 +48,7 @@ func (s *FakeService) AddTeamMember(userID, orgID, teamID int64, isExternal bool return s.ExpectedError } -func (s *FakeService) UpdateTeamMember(ctx context.Context, cmd *models.UpdateTeamMemberCommand) error { +func (s *FakeService) UpdateTeamMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error { return s.ExpectedError } @@ -56,16 +56,16 @@ func (s *FakeService) IsTeamMember(orgId int64, teamId int64, userId int64) (boo return s.ExpectedIsMember, s.ExpectedError } -func (s *FakeService) RemoveTeamMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error { +func (s *FakeService) RemoveTeamMember(ctx context.Context, cmd *team.RemoveTeamMemberCommand) error { return s.ExpectedError } -func (s *FakeService) GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error) { +func (s *FakeService) GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*team.TeamMemberDTO, error) { return s.ExpectedMembers, s.ExpectedError } -func (s *FakeService) GetTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery) error { - return s.ExpectedError +func (s *FakeService) GetTeamMembers(ctx context.Context, query *team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) { + return s.ExpectedMembers, s.ExpectedError } func (s *FakeService) IsAdminOfTeams(ctx context.Context, query *team.IsAdminOfTeamsQuery) (bool, error) { diff --git a/pkg/services/teamguardian/database/database.go b/pkg/services/teamguardian/database/database.go index b58c4efc2a4..4ddc50a4cc2 100644 --- a/pkg/services/teamguardian/database/database.go +++ b/pkg/services/teamguardian/database/database.go @@ -4,7 +4,6 @@ import ( "context" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/team" ) @@ -17,12 +16,13 @@ func ProvideTeamGuardianStore(sqlStore db.DB, teamService team.Service) *TeamGua return &TeamGuardianStoreImpl{sqlStore: sqlStore, teamService: teamService} } -func (t *TeamGuardianStoreImpl) GetTeamMembers(ctx context.Context, query models.GetTeamMembersQuery) ([]*models.TeamMemberDTO, error) { - if err := t.teamService.GetTeamMembers(ctx, &query); err != nil { +func (t *TeamGuardianStoreImpl) GetTeamMembers(ctx context.Context, query team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) { + queryResult, err := t.teamService.GetTeamMembers(ctx, &query) + if err != nil { return nil, err } - return query.Result, nil + return queryResult, nil } func (t *TeamGuardianStoreImpl) DeleteByUser(ctx context.Context, userID int64) error { diff --git a/pkg/services/teamguardian/database/database_mock.go b/pkg/services/teamguardian/database/database_mock.go index 24807fc83d9..3b58cc06ebb 100644 --- a/pkg/services/teamguardian/database/database_mock.go +++ b/pkg/services/teamguardian/database/database_mock.go @@ -3,7 +3,7 @@ package database import ( "context" - "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/team" "github.com/stretchr/testify/mock" ) @@ -11,9 +11,9 @@ type TeamGuardianStoreMock struct { mock.Mock } -func (t *TeamGuardianStoreMock) GetTeamMembers(ctx context.Context, query models.GetTeamMembersQuery) ([]*models.TeamMemberDTO, error) { +func (t *TeamGuardianStoreMock) GetTeamMembers(ctx context.Context, query team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) { args := t.Called(ctx, query) - return args.Get(0).([]*models.TeamMemberDTO), args.Error(1) + return args.Get(0).([]*team.TeamMemberDTO), args.Error(1) } func (t *TeamGuardianStoreMock) DeleteByUser(ctx context.Context, userID int64) error { diff --git a/pkg/services/teamguardian/manager/service.go b/pkg/services/teamguardian/manager/service.go index 394dcb5cbe5..9c60116c639 100644 --- a/pkg/services/teamguardian/manager/service.go +++ b/pkg/services/teamguardian/manager/service.go @@ -27,10 +27,10 @@ func (s *Service) CanAdmin(ctx context.Context, orgId int64, teamId int64, user return team.ErrNotAllowedToUpdateTeamInDifferentOrg } - cmd := models.GetTeamMembersQuery{ - OrgId: orgId, - TeamId: teamId, - UserId: user.UserID, + cmd := team.GetTeamMembersQuery{ + OrgID: orgId, + TeamID: teamId, + UserID: user.UserID, SignedInUser: user, } @@ -40,7 +40,7 @@ func (s *Service) CanAdmin(ctx context.Context, orgId int64, teamId int64, user } for _, member := range results { - if member.UserId == user.UserID && member.Permission == models.PERMISSION_ADMIN { + if member.UserID == user.UserID && member.Permission == models.PERMISSION_ADMIN { return nil } } diff --git a/pkg/services/teamguardian/manager/service_test.go b/pkg/services/teamguardian/manager/service_test.go index d50878d0fd3..abde850dadb 100644 --- a/pkg/services/teamguardian/manager/service_test.go +++ b/pkg/services/teamguardian/manager/service_test.go @@ -36,7 +36,7 @@ func TestUpdateTeam(t *testing.T) { t.Run("Given an editor and a team he isn't a member of", func(t *testing.T) { t.Run("Should not be able to update the team", func(t *testing.T) { ctx := context.Background() - store.On("GetTeamMembers", ctx, mock.Anything).Return([]*models.TeamMemberDTO{}, nil).Once() + store.On("GetTeamMembers", ctx, mock.Anything).Return([]*team.TeamMemberDTO{}, nil).Once() err := teamGuardianService.CanAdmin(ctx, testTeam.OrgID, testTeam.ID, &editor) require.Equal(t, team.ErrNotAllowedToUpdateTeam, err) }) @@ -46,10 +46,10 @@ func TestUpdateTeam(t *testing.T) { t.Run("Should be able to update the team", func(t *testing.T) { ctx := context.Background() - result := []*models.TeamMemberDTO{{ - OrgId: testTeam.OrgID, - TeamId: testTeam.ID, - UserId: editor.UserID, + result := []*team.TeamMemberDTO{{ + OrgID: testTeam.OrgID, + TeamID: testTeam.ID, + UserID: editor.UserID, Permission: models.PERMISSION_ADMIN, }} @@ -68,10 +68,10 @@ func TestUpdateTeam(t *testing.T) { } t.Run("Shouldn't be able to update the team", func(t *testing.T) { - result := []*models.TeamMemberDTO{{ - OrgId: testTeamOtherOrg.OrgID, - TeamId: testTeamOtherOrg.ID, - UserId: editor.UserID, + result := []*team.TeamMemberDTO{{ + OrgID: testTeamOtherOrg.OrgID, + TeamID: testTeamOtherOrg.ID, + UserID: editor.UserID, Permission: models.PERMISSION_ADMIN, }} diff --git a/pkg/services/teamguardian/team.go b/pkg/services/teamguardian/team.go index 497d5d89f9d..7c930deb58b 100644 --- a/pkg/services/teamguardian/team.go +++ b/pkg/services/teamguardian/team.go @@ -3,7 +3,7 @@ package teamguardian import ( "context" - "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" ) @@ -13,6 +13,6 @@ type TeamGuardian interface { } type Store interface { - GetTeamMembers(context.Context, models.GetTeamMembersQuery) ([]*models.TeamMemberDTO, error) + GetTeamMembers(context.Context, team.GetTeamMembersQuery) ([]*team.TeamMemberDTO, error) DeleteByUser(context.Context, int64) error }