From 748b3c855c3963ab6ffd824a4ca7fe4771ef88f7 Mon Sep 17 00:00:00 2001 From: Ieva Date: Thu, 25 Apr 2024 16:46:24 +0100 Subject: [PATCH] Chore: Clean up team membership code (#86914) remove unused code, clean up commands --- pkg/services/team/model.go | 8 ++---- pkg/services/team/team.go | 5 ---- pkg/services/team/teamapi/team_members.go | 7 +++--- pkg/services/team/teamimpl/store.go | 30 ----------------------- pkg/services/team/teamimpl/team.go | 13 ---------- pkg/services/team/teamtest/team.go | 13 ---------- 6 files changed, 5 insertions(+), 71 deletions(-) diff --git a/pkg/services/team/model.go b/pkg/services/team/model.go index a4d4fe0a6a6..0426368de17 100644 --- a/pkg/services/team/model.go +++ b/pkg/services/team/model.go @@ -21,6 +21,8 @@ var ( ErrTeamMemberAlreadyAdded = errors.New("user is already added to this team") ) +const MemberPermissionName = "Member" + // Team model type Team struct { ID int64 `json:"id" xorm:"pk autoincr 'id'"` @@ -124,16 +126,10 @@ type TeamMember struct { type AddTeamMemberCommand struct { UserID int64 `json:"userId" binding:"Required"` - OrgID int64 `json:"-"` - TeamID int64 `json:"-"` - External bool `json:"-"` Permission dashboardaccess.PermissionType `json:"-"` } type UpdateTeamMemberCommand struct { - UserID int64 `json:"-"` - OrgID int64 `json:"-"` - TeamID int64 `json:"-"` Permission dashboardaccess.PermissionType `json:"permission"` } diff --git a/pkg/services/team/team.go b/pkg/services/team/team.go index 5f1bca40244..662f2136ae4 100644 --- a/pkg/services/team/team.go +++ b/pkg/services/team/team.go @@ -2,8 +2,6 @@ package team import ( "context" - - "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" ) type Service interface { @@ -14,10 +12,7 @@ type Service interface { GetTeamByID(ctx context.Context, query *GetTeamByIDQuery) (*TeamDTO, error) GetTeamsByUser(ctx context.Context, query *GetTeamsByUserQuery) ([]*TeamDTO, error) GetTeamIDsByUser(ctx context.Context, query *GetTeamIDsByUserQuery) ([]int64, error) - AddTeamMember(ctx context.Context, userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error - UpdateTeamMember(ctx context.Context, cmd *UpdateTeamMemberCommand) error IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error) - RemoveTeamMember(ctx context.Context, cmd *RemoveTeamMemberCommand) error RemoveUsersMemberships(tx context.Context, userID int64) error GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*TeamMemberDTO, error) GetTeamMembers(ctx context.Context, query *GetTeamMembersQuery) ([]*TeamMemberDTO, error) diff --git a/pkg/services/team/teamapi/team_members.go b/pkg/services/team/teamapi/team_members.go index 89419b8ff09..e5b0b79e9e5 100644 --- a/pkg/services/team/teamapi/team_members.go +++ b/pkg/services/team/teamapi/team_members.go @@ -77,13 +77,12 @@ func (tapi *TeamAPI) addTeamMember(c *contextmodel.ReqContext) response.Response if err := web.Bind(c.Req, &cmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } - cmd.OrgID = c.SignedInUser.GetOrgID() - cmd.TeamID, err = strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64) + teamID, err := strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64) if err != nil { return response.Error(http.StatusBadRequest, "teamId is invalid", err) } - isTeamMember, err := tapi.teamService.IsTeamMember(c.SignedInUser.GetOrgID(), cmd.TeamID, cmd.UserID) + isTeamMember, err := tapi.teamService.IsTeamMember(c.SignedInUser.GetOrgID(), teamID, cmd.UserID) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to add team member.", err) } @@ -91,7 +90,7 @@ func (tapi *TeamAPI) addTeamMember(c *contextmodel.ReqContext) response.Response return response.Error(http.StatusBadRequest, "User is already added to this team", nil) } - err = addOrUpdateTeamMember(c.Req.Context(), tapi.teamPermissionsService, cmd.UserID, cmd.OrgID, cmd.TeamID, getPermissionName(cmd.Permission)) + err = addOrUpdateTeamMember(c.Req.Context(), tapi.teamPermissionsService, cmd.UserID, c.SignedInUser.GetOrgID(), teamID, team.MemberPermissionName) if err != nil { return response.Error(http.StatusInternalServerError, "Failed to add Member to Team", err) } diff --git a/pkg/services/team/teamimpl/store.go b/pkg/services/team/teamimpl/store.go index f8a85618d99..2076c5cf95d 100644 --- a/pkg/services/team/teamimpl/store.go +++ b/pkg/services/team/teamimpl/store.go @@ -26,10 +26,7 @@ type store interface { GetByUser(ctx context.Context, query *team.GetTeamsByUserQuery) ([]*team.TeamDTO, error) GetIDsByUser(ctx context.Context, query *team.GetTeamIDsByUserQuery) ([]int64, error) RemoveUsersMemberships(ctx context.Context, userID int64) error - AddMember(ctx context.Context, userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error - UpdateMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error IsMember(orgId int64, teamId int64, userId int64) (bool, 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) RegisterDelete(query string) @@ -350,19 +347,6 @@ WHERE tm.user_id=? AND tm.org_id=?;`, query.UserID, query.OrgID).Find(&queryResu return queryResult, nil } -// AddTeamMember adds a user to a team -func (ss *xormStore) AddMember(ctx context.Context, userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error { - return ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { - if isMember, err := isTeamMember(sess, orgID, teamID, userID); err != nil { - return err - } else if isMember { - return team.ErrTeamMemberAlreadyAdded - } - - return addTeamMember(sess, orgID, teamID, userID, isExternal, permission) - }) -} - 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 team.TeamMember @@ -378,13 +362,6 @@ func getTeamMember(sess *db.Session, orgId int64, teamId int64, userId int64) (t return member, nil } -// UpdateTeamMember updates a team member -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) - }) -} - func (ss *xormStore) IsMember(orgId int64, teamId int64, userId int64) (bool, error) { var isMember bool @@ -458,13 +435,6 @@ func updateTeamMember(sess *db.Session, orgID, teamID, userID int64, permission return err } -// RemoveTeamMember removes a member from a team -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) - }) -} - // 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 *team.RemoveTeamMemberCommand) error { diff --git a/pkg/services/team/teamimpl/team.go b/pkg/services/team/teamimpl/team.go index 11cef2ab58e..18e29595213 100644 --- a/pkg/services/team/teamimpl/team.go +++ b/pkg/services/team/teamimpl/team.go @@ -4,7 +4,6 @@ import ( "context" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/setting" ) @@ -50,22 +49,10 @@ func (s *Service) GetTeamIDsByUser(ctx context.Context, query *team.GetTeamIDsBy return s.store.GetIDsByUser(ctx, query) } -func (s *Service) AddTeamMember(ctx context.Context, userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error { - return s.store.AddMember(ctx, userID, orgID, teamID, isExternal, permission) -} - -func (s *Service) UpdateTeamMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error { - return s.store.UpdateMember(ctx, cmd) -} - func (s *Service) IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error) { return s.store.IsMember(orgId, teamId, userId) } -func (s *Service) RemoveTeamMember(ctx context.Context, cmd *team.RemoveTeamMemberCommand) error { - return s.store.RemoveMember(ctx, cmd) -} - func (s *Service) RemoveUsersMemberships(ctx context.Context, userID int64) error { return s.store.RemoveUsersMemberships(ctx, userID) } diff --git a/pkg/services/team/teamtest/team.go b/pkg/services/team/teamtest/team.go index 2c6fccdf333..58e9fda9750 100644 --- a/pkg/services/team/teamtest/team.go +++ b/pkg/services/team/teamtest/team.go @@ -3,7 +3,6 @@ package teamtest import ( "context" - "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/team" ) @@ -45,22 +44,10 @@ func (s *FakeService) GetTeamsByUser(ctx context.Context, query *team.GetTeamsBy return s.ExpectedTeamsByUser, s.ExpectedError } -func (s *FakeService) AddTeamMember(ctx context.Context, userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error { - return s.ExpectedError -} - -func (s *FakeService) UpdateTeamMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error { - return s.ExpectedError -} - func (s *FakeService) IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error) { return s.ExpectedIsMember, s.ExpectedError } -func (s *FakeService) RemoveTeamMember(ctx context.Context, cmd *team.RemoveTeamMemberCommand) error { - return s.ExpectedError -} - func (s *FakeService) RemoveUsersMemberships(ctx context.Context, userID int64) error { return s.ExpectedError }