From 37f549261a782224f6bbf72d0fe412ef35b43679 Mon Sep 17 00:00:00 2001 From: Bradyn Poulsen Date: Tue, 13 Sep 2016 16:52:43 -0600 Subject: [PATCH 1/4] cascades the maintainers team from the configuration --- cache/helper.go | 6 +++--- remote/github/github.go | 6 +++--- web/hook.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cache/helper.go b/cache/helper.go index 7cee5d1..4e7802a 100644 --- a/cache/helper.go +++ b/cache/helper.go @@ -75,9 +75,9 @@ func GetPerm(c context.Context, user *model.User, owner, name string) (*model.Pe } // GetMembers returns the team members from the cache. -func GetMembers(c context.Context, user *model.User, team string) ([]*model.Member, error) { +func GetMembers(c context.Context, user *model.User, owner string, maintainers string) ([]*model.Member, error) { key := fmt.Sprintf("members:%s", - team, + owner, ) // if we fetch from the cache we can return immediately val, err := FromContext(c).Get(key) @@ -86,7 +86,7 @@ func GetMembers(c context.Context, user *model.User, team string) ([]*model.Memb } // else we try to grab from the remote system and // populate our cache. - members, err := remote.GetMembers(c, user, team) + members, err := remote.GetMembers(c, user, owner, maintainers) if err != nil { return nil, err } diff --git a/remote/github/github.go b/remote/github/github.go index 5dc79e4..00176a8 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -93,15 +93,15 @@ func (g *Github) GetTeams(user *model.User) ([]*model.Team, error) { return teams, nil } -func (g *Github) GetMembers(user *model.User, team string) ([]*model.Member, error) { +func (g *Github) GetMembers(user *model.User, owner string, maintainers string) ([]*model.Member, error) { client := setupClient(g.API, user.Token) - teams, _, err := client.Organizations.ListTeams(team, &github.ListOptions{PerPage: 100}) + teams, _, err := client.Organizations.ListTeams(owner, &github.ListOptions{PerPage: 100}) if err != nil { return nil, fmt.Errorf("Error accessing team list. %s", err) } var id int for _, team := range teams { - if strings.ToLower(*team.Name) == "maintainers" { + if strings.ToLower(*team.Name) == strings.ToLower(maintainers) { id = *team.ID break } diff --git a/web/hook.go b/web/hook.go index b4976ec..a92ee0f 100644 --- a/web/hook.go +++ b/web/hook.go @@ -49,7 +49,7 @@ func Hook(c *gin.Context) { file, err := remote.GetContents(c, user, repo, "MAINTAINERS") if err != nil { log.Debugf("no MAINTAINERS file for %s. Checking for team members.", repo.Slug) - members, merr := cache.GetMembers(c, user, repo.Owner) + members, merr := cache.GetMembers(c, user, repo.Owner, rcfile.Team) if merr != nil { log.Errorf("Error getting repository %s. %s", repo.Slug, err) log.Errorf("Error getting org members %s. %s", repo.Owner, merr) From d07e8f296a6e8294cc3b8eded8c70abce02b48e3 Mon Sep 17 00:00:00 2001 From: Bradyn Poulsen Date: Tue, 13 Sep 2016 16:55:13 -0600 Subject: [PATCH 2/4] update the cache helper tests to pass the maintainers team value --- cache/helper_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cache/helper_test.go b/cache/helper_test.go index c0478e1..ca21158 100644 --- a/cache/helper_test.go +++ b/cache/helper_test.go @@ -112,8 +112,8 @@ func TestHelper(t *testing.T) { }) g.It("Should set and get members", func() { - r.On("GetMembers", fakeUser, "drone").Return(fakeMembers, nil).Once() - p, err := GetMembers(c, fakeUser, "drone") + r.On("GetMembers", fakeUser, "drone", "maintainers").Return(fakeMembers, nil).Once() + p, err := GetMembers(c, fakeUser, "drone", "maintainers") g.Assert(p).Equal(fakeMembers) g.Assert(err).Equal(nil) }) @@ -122,15 +122,15 @@ func TestHelper(t *testing.T) { key := "members:drone" Set(c, key, fakeMembers) - r.On("GetMembers", fakeUser, "drone").Return(nil, fakeErr).Once() - p, err := GetMembers(c, fakeUser, "drone") + r.On("GetMembers", fakeUser, "drone", "maintainers").Return(nil, fakeErr).Once() + p, err := GetMembers(c, fakeUser, "drone", "maintainers") g.Assert(p).Equal(fakeMembers) g.Assert(err).Equal(nil) }) g.It("Should get member error", func() { - r.On("GetMembers", fakeUser, "drone").Return(nil, fakeErr).Once() - p, err := GetMembers(c, fakeUser, "drone") + r.On("GetMembers", fakeUser, "drone", "maintainers").Return(nil, fakeErr).Once() + p, err := GetMembers(c, fakeUser, "drone", "maintainers") g.Assert(p == nil).IsTrue() g.Assert(err).Equal(fakeErr) }) From 92acd2f746fac4e6f3c9c949fd6e46e0cb7ef891 Mon Sep 17 00:00:00 2001 From: Bradyn Poulsen Date: Tue, 13 Sep 2016 17:01:41 -0600 Subject: [PATCH 3/4] adjust the mock remote for GetMembers --- remote/mock/remote.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/remote/mock/remote.go b/remote/mock/remote.go index 8152cb4..8f0404e 100644 --- a/remote/mock/remote.go +++ b/remote/mock/remote.go @@ -96,12 +96,12 @@ func (_m *Remote) GetHook(r *http.Request) (*model.Hook, error) { } // GetMembers provides a mock function with given fields: _a0, _a1 -func (_m *Remote) GetMembers(_a0 *model.User, _a1 string) ([]*model.Member, error) { - ret := _m.Called(_a0, _a1) +func (_m *Remote) GetMembers(_a0 *model.User, _a1 string, _a2 string) ([]*model.Member, error) { + ret := _m.Called(_a0, _a1, _a2) var r0 []*model.Member - if rf, ok := ret.Get(0).(func(*model.User, string) []*model.Member); ok { - r0 = rf(_a0, _a1) + if rf, ok := ret.Get(0).(func(*model.User, string, string) []*model.Member); ok { + r0 = rf(_a0, _a1, _a2) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.Member) @@ -109,8 +109,8 @@ func (_m *Remote) GetMembers(_a0 *model.User, _a1 string) ([]*model.Member, erro } var r1 error - if rf, ok := ret.Get(1).(func(*model.User, string) error); ok { - r1 = rf(_a0, _a1) + if rf, ok := ret.Get(1).(func(*model.User, string, string) error); ok { + r1 = rf(_a0, _a1, _a2) } else { r1 = ret.Error(1) } From aeea938a2d2aa17fa52aa9a8217d4122f4186d83 Mon Sep 17 00:00:00 2001 From: crandl201 Date: Tue, 8 Nov 2016 15:53:32 -0500 Subject: [PATCH 4/4] Fix build issues; update api/maintainer to use the Team config as well --- api/maintainer.go | 11 ++++++++++- remote/remote.go | 6 +++--- web/hook.go | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/api/maintainer.go b/api/maintainer.go index a8e7de9..c561627 100644 --- a/api/maintainer.go +++ b/api/maintainer.go @@ -24,10 +24,19 @@ func GetMaintainer(c *gin.Context) { c.AbortWithStatus(404) return } + + rcfile, _ := remote.GetContents(c, user, repo, ".lgtm") + config, err := model.ParseConfig(rcfile) + if err != nil { + log.Errorf("Error parsing .lgtm file for %s. %s", repo.Slug, err) + c.String(500, "Error parsing .lgtm file. %s.", err) + return + } + file, err := remote.GetContents(c, user, repo, "MAINTAINERS") if err != nil { log.Debugf("no MAINTAINERS file for %s. Checking for team members.", repo.Slug) - members, merr := cache.GetMembers(c, user, repo.Owner) + members, merr := cache.GetMembers(c, user, repo.Owner, config.Team) if merr != nil { log.Errorf("Error getting repository %s. %s", repo.Slug, err) log.Errorf("Error getting org members %s. %s", repo.Owner, merr) diff --git a/remote/remote.go b/remote/remote.go index bbe9627..a46cd6a 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -21,7 +21,7 @@ type Remote interface { GetTeams(*model.User) ([]*model.Team, error) // GetMembers gets a team member list from the remote system. - GetMembers(*model.User, string) ([]*model.Member, error) + GetMembers(*model.User, string, string) ([]*model.Member, error) // GetRepo gets a repository from the remote system. GetRepo(*model.User, string, string) (*model.Repo, error) @@ -68,8 +68,8 @@ func GetTeams(c context.Context, u *model.User) ([]*model.Team, error) { } // GetMembers gets a team members list from the remote system. -func GetMembers(c context.Context, u *model.User, team string) ([]*model.Member, error) { - return FromContext(c).GetMembers(u, team) +func GetMembers(c context.Context, u *model.User, owner string, maintainers string) ([]*model.Member, error) { + return FromContext(c).GetMembers(u, owner, maintainers) } // GetRepo gets a repository from the remote system. diff --git a/web/hook.go b/web/hook.go index a92ee0f..10887d3 100644 --- a/web/hook.go +++ b/web/hook.go @@ -49,7 +49,7 @@ func Hook(c *gin.Context) { file, err := remote.GetContents(c, user, repo, "MAINTAINERS") if err != nil { log.Debugf("no MAINTAINERS file for %s. Checking for team members.", repo.Slug) - members, merr := cache.GetMembers(c, user, repo.Owner, rcfile.Team) + members, merr := cache.GetMembers(c, user, repo.Owner, config.Team) if merr != nil { log.Errorf("Error getting repository %s. %s", repo.Slug, err) log.Errorf("Error getting org members %s. %s", repo.Owner, merr)