Skip to content

Commit a1c44fa

Browse files
jenkins-x-botjenkins-x-bot
authored
Merge pull request #199 from abayer/gitlab-pr-no-repo
fix: Fetch repo info when converting GitLab PRs
2 parents 25041a8 + 26cff67 commit a1c44fa

File tree

14 files changed

+307
-69
lines changed

14 files changed

+307
-69
lines changed

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ require (
66
github.com/ghodss/yaml v1.0.0
77
github.com/google/go-cmp v0.3.0
88
github.com/h2non/gock v1.0.9
9+
github.com/mitchellh/copystructure v1.0.0
910
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32 // indirect
1011
github.com/pkg/errors v0.8.1
1112
github.com/shurcooL/githubv4 v0.0.0-20190718010115-4ba037080260

‎go.sum‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN
3535
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
3636
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
3737
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
38+
github.com/mitchellh/copystructure v1.0.0 h1:Laisrj+bAB6b/yJwB5Bt3ITZhGJdqmxquMKeZ+mmkFQ=
39+
github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw=
40+
github.com/mitchellh/reflectwalk v1.0.0 h1:9D+8oIskB4VJBN5SFlmc27fSlIBZaov1Wpk/IfikLNY=
41+
github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
3842
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
3943
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
4044
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=

‎scm/driver/gitlab/content.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,13 @@ type updateContentBody struct {
110110
}
111111

112112
type entry struct {
113-
Id string `json:"id"`
113+
Id string `json:"id"`
114114
Name string `json:"name"`
115115
Type string `json:"type"`
116116
Path string `json:"path"`
117117
Mode string `json:"mode"`
118118
}
119119

120-
121120
func convertEntryList(out []*entry) []*scm.FileEntry {
122121
answer := make([]*scm.FileEntry, 0, len(out))
123122
for _, o := range out {

‎scm/driver/gitlab/pr.go‎

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ package gitlab
77
import (
88
"context"
99
"fmt"
10-
"github.com/sirupsen/logrus"
10+
"github.com/mitchellh/copystructure"
1111
"net/url"
1212
"strconv"
1313
"strings"
@@ -24,7 +24,14 @@ func (s *pullService) Find(ctx context.Context, repo string, number int) (*scm.P
2424
path := fmt.Sprintf("api/v4/projects/%s/merge_requests/%d", encode(repo), number)
2525
out := new(pr)
2626
res, err := s.client.do(ctx, "GET", path, nil, out)
27-
return convertPullRequest(out, repo), res, err
27+
if err != nil {
28+
return nil, res, err
29+
}
30+
convRepo, convRes, err := s.convertPullRequest(ctx, out)
31+
if err != nil {
32+
return nil, convRes, err
33+
}
34+
return convRepo, res, nil
2835
}
2936

3037
func (s *pullService) FindComment(ctx context.Context, repo string, index, id int) (*scm.Comment, *scm.Response, error) {
@@ -38,7 +45,14 @@ func (s *pullService) List(ctx context.Context, repo string, opts scm.PullReques
3845
path := fmt.Sprintf("api/v4/projects/%s/merge_requests?%s", encode(repo), encodePullRequestListOptions(opts))
3946
out := []*pr{}
4047
res, err := s.client.do(ctx, "GET", path, nil, &out)
41-
return convertPullRequestList(out, repo), res, err
48+
if err != nil {
49+
return nil, res, err
50+
}
51+
convRepos, convRes, err := s.convertPullRequestList(ctx, out)
52+
if err != nil {
53+
return nil, convRes, err
54+
}
55+
return convRepos, res, nil
4256
}
4357

4458
func (s *pullService) ListChanges(ctx context.Context, repo string, number int, opts scm.ListOptions) ([]*scm.Change, *scm.Response, error) {
@@ -50,7 +64,6 @@ func (s *pullService) ListChanges(ctx context.Context, repo string, number int,
5064

5165
func (s *pullService) ListComments(ctx context.Context, repo string, index int, opts scm.ListOptions) ([]*scm.Comment, *scm.Response, error) {
5266
path := fmt.Sprintf("api/v4/projects/%s/merge_requests/%d/notes?%s", encode(repo), index, encodeListOptions(opts))
53-
logrus.Errorf("path: %s", path)
5467
out := []*issueComment{}
5568
res, err := s.client.do(ctx, "GET", path, nil, &out)
5669
return convertIssueCommentList(out), res, err
@@ -234,7 +247,14 @@ func (s *pullService) Create(ctx context.Context, repo string, input *scm.PullRe
234247

235248
out := new(pr)
236249
res, err := s.client.do(ctx, "POST", path, in, out)
237-
return convertPullRequest(out, repo), res, err
250+
if err != nil {
251+
return nil, res, err
252+
}
253+
convRepo, convRes, err := s.convertPullRequest(ctx, out)
254+
if err != nil {
255+
return nil, convRes, err
256+
}
257+
return convRepo, res, nil
238258
}
239259

240260
func (s *pullService) Update(ctx context.Context, repo string, number int, input *scm.PullRequestInput) (*scm.PullRequest, *scm.Response, error) {
@@ -288,7 +308,14 @@ func (s *pullService) updateMergeRequestField(ctx context.Context, repo string,
288308

289309
out := new(pr)
290310
res, err := s.client.do(ctx, "PUT", path, input, out)
291-
return convertPullRequest(out, repo), res, err
311+
if err != nil {
312+
return nil, res, err
313+
}
314+
convRepo, convRes, err := s.convertPullRequest(ctx, out)
315+
if err != nil {
316+
return nil, convRes, err
317+
}
318+
return convRepo, res, nil
292319
}
293320

294321
type pr struct {
@@ -346,16 +373,19 @@ type pullRequestMergeRequest struct {
346373
MergeWhenPipelineSucceeds string `json:"merge_when_pipeline_succeeds,omitempty"`
347374
}
348375

349-
func convertPullRequestList(from []*pr, repo string) []*scm.PullRequest {
376+
func (s *pullService) convertPullRequestList(ctx context.Context, from []*pr) ([]*scm.PullRequest, *scm.Response, error) {
350377
to := []*scm.PullRequest{}
351378
for _, v := range from {
352-
to = append(to, convertPullRequest(v, repo))
379+
converted, res, err := s.convertPullRequest(ctx, v)
380+
if err != nil {
381+
return nil, res, err
382+
}
383+
to = append(to, converted)
353384
}
354-
return to
385+
return to, nil, nil
355386
}
356387

357-
func convertPullRequest(from *pr, repo string) *scm.PullRequest {
358-
repoNS, repoName := scm.Split(repo)
388+
func (s *pullService) convertPullRequest(ctx context.Context, from *pr) (*scm.PullRequest, *scm.Response, error) {
359389
// Diff refs only seem to be populated in more recent merge requests. Default
360390
// to from.Sha for compatibility / consistency, but fallback to HeadSHA if
361391
// it's not populated.
@@ -370,6 +400,24 @@ func convertPullRequest(from *pr, repo string) *scm.PullRequest {
370400
for _, a := range from.Assignees {
371401
assignees = append(assignees, *convertUser(a))
372402
}
403+
var res *scm.Response
404+
baseRepo, res, err := s.client.Repositories.Find(ctx, strconv.Itoa(from.TargetProjectID))
405+
if err != nil {
406+
return nil, res, err
407+
}
408+
var headRepo *scm.Repository
409+
if from.TargetProjectID == from.SourceProjectID {
410+
repoCopy, err := copystructure.Copy(baseRepo)
411+
if err != nil {
412+
return nil, nil, err
413+
}
414+
headRepo = repoCopy.(*scm.Repository)
415+
} else {
416+
headRepo, res, err = s.client.Repositories.Find(ctx, strconv.Itoa(from.SourceProjectID))
417+
if err != nil {
418+
return nil, res, err
419+
}
420+
}
373421
return &scm.PullRequest{
374422
Number: from.Number,
375423
Title: from.Title,
@@ -389,25 +437,18 @@ func convertPullRequest(from *pr, repo string) *scm.PullRequest {
389437
Author: *convertUser(&from.Author),
390438
Assignees: assignees,
391439
Head: scm.PullRequestBranch{
392-
Ref: from.SourceBranch,
393-
Sha: headSHA,
394-
Repo: scm.Repository{
395-
ID: strconv.Itoa(from.SourceProjectID),
396-
},
440+
Ref: from.SourceBranch,
441+
Sha: headSHA,
442+
Repo: *headRepo,
397443
},
398444
Base: scm.PullRequestBranch{
399-
Ref: from.TargetBranch,
400-
Sha: from.DiffRefs.BaseSHA,
401-
Repo: scm.Repository{
402-
ID: strconv.Itoa(from.TargetProjectID),
403-
Name: repoName,
404-
Namespace: repoNS,
405-
FullName: repo,
406-
},
445+
Ref: from.TargetBranch,
446+
Sha: from.DiffRefs.BaseSHA,
447+
Repo: *baseRepo,
407448
},
408449
Created: from.Created,
409450
Updated: from.Updated,
410-
}
451+
}, nil, nil
411452
}
412453

413454
func convertPullRequestLabels(from []*string) []*scm.Label {

‎scm/driver/gitlab/pr_test.go‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ import (
1919
func TestPullFind(t *testing.T) {
2020
defer gock.Off()
2121

22+
gock.New("https://gitlab.com").
23+
Get("/api/v4/projects/32732").
24+
Reply(200).
25+
Type("application/json").
26+
SetHeaders(mockHeaders).
27+
File("testdata/repo.json")
28+
2229
gock.New("https://gitlab.com").
2330
Get("/api/v4/projects/diaspora/diaspora/merge_requests/1347").
2431
Reply(200).
@@ -42,7 +49,7 @@ func TestPullFind(t *testing.T) {
4249
t.Fatalf("json.Unmarshal: %v", err)
4350
}
4451

45-
if diff := cmp.Diff(got, want); diff != "" {
52+
if diff := cmp.Diff(want, got); diff != "" {
4653
t.Errorf("Unexpected Results")
4754
t.Log(diff)
4855
}
@@ -54,6 +61,13 @@ func TestPullFind(t *testing.T) {
5461
func TestPullList(t *testing.T) {
5562
defer gock.Off()
5663

64+
gock.New("https://gitlab.com").
65+
Get("/api/v4/projects/32732").
66+
Reply(200).
67+
Type("application/json").
68+
SetHeaders(mockHeaders).
69+
File("testdata/repo.json")
70+
5771
updatedAfter, _ := time.Parse(scm.SearchTimeFormat, "2015-12-18T17:30:22.522Z")
5872
gock.New("https://gitlab.com").
5973
Get("/api/v4/projects/diaspora/diaspora/merge_requests").
@@ -349,6 +363,20 @@ func TestPullEditComment(t *testing.T) {
349363
func TestPullCreate(t *testing.T) {
350364
defer gock.Off()
351365

366+
gock.New("https://gitlab.com").
367+
Get("/api/v4/projects/32732").
368+
Reply(200).
369+
Type("application/json").
370+
SetHeaders(mockHeaders).
371+
File("testdata/repo.json")
372+
373+
gock.New("https://gitlab.com").
374+
Get("/api/v4/projects/2").
375+
Reply(200).
376+
Type("application/json").
377+
SetHeaders(mockHeaders).
378+
File("testdata/other_repo.json")
379+
352380
gock.New("https://gitlab.com").
353381
Post("/api/v4/projects/diaspora/diaspora/merge_requests").
354382
Reply(201).
@@ -373,7 +401,7 @@ func TestPullCreate(t *testing.T) {
373401
raw, _ := ioutil.ReadFile("testdata/pr_create.json.golden")
374402
json.Unmarshal(raw, want)
375403

376-
if diff := cmp.Diff(got, want); diff != "" {
404+
if diff := cmp.Diff(want, got); diff != "" {
377405
t.Errorf("Unexpected Results")
378406
t.Log(diff)
379407
}
@@ -385,6 +413,20 @@ func TestPullCreate(t *testing.T) {
385413
func TestPullUpdate(t *testing.T) {
386414
defer gock.Off()
387415

416+
gock.New("https://gitlab.com").
417+
Get("/api/v4/projects/32732").
418+
Reply(200).
419+
Type("application/json").
420+
SetHeaders(mockHeaders).
421+
File("testdata/repo.json")
422+
423+
gock.New("https://gitlab.com").
424+
Get("/api/v4/projects/2").
425+
Reply(200).
426+
Type("application/json").
427+
SetHeaders(mockHeaders).
428+
File("testdata/other_repo.json")
429+
388430
gock.New("https://gitlab.com").
389431
Put("/api/v4/projects/diaspora/diaspora/merge_requests/1").
390432
File("testdata/pr_update.json").
@@ -457,6 +499,20 @@ func TestPullListEvents(t *testing.T) {
457499
func TestPullSetMilestone(t *testing.T) {
458500
defer gock.Off()
459501

502+
gock.New("https://gitlab.com").
503+
Get("/api/v4/projects/32732").
504+
Reply(200).
505+
Type("application/json").
506+
SetHeaders(mockHeaders).
507+
File("testdata/repo.json")
508+
509+
gock.New("https://gitlab.com").
510+
Get("/api/v4/projects/2").
511+
Reply(200).
512+
Type("application/json").
513+
SetHeaders(mockHeaders).
514+
File("testdata/other_repo.json")
515+
460516
gock.New("https://gitlab.com").
461517
Put("/api/v4/projects/diaspora/diaspora/merge_requests/1").
462518
File("testdata/issue_or_pr_set_milestone.json").
@@ -475,6 +531,20 @@ func TestPullSetMilestone(t *testing.T) {
475531
func TestPullClearMilestone(t *testing.T) {
476532
defer gock.Off()
477533

534+
gock.New("https://gitlab.com").
535+
Get("/api/v4/projects/32732").
536+
Reply(200).
537+
Type("application/json").
538+
SetHeaders(mockHeaders).
539+
File("testdata/repo.json")
540+
541+
gock.New("https://gitlab.com").
542+
Get("/api/v4/projects/2").
543+
Reply(200).
544+
Type("application/json").
545+
SetHeaders(mockHeaders).
546+
File("testdata/other_repo.json")
547+
478548
gock.New("https://gitlab.com").
479549
Put("/api/v4/projects/diaspora/diaspora/merge_requests/1").
480550
File("testdata/issue_or_pr_clear_milestone.json").

‎scm/driver/gitlab/testdata/merge.json.golden‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,43 @@
3232
"Ref": "fix",
3333
"Sha": "12d65c8dd2b2676fa3ac47d955accc085a37a9c1",
3434
"Repo": {
35-
"ID": "32732"
35+
"ID": "32732",
36+
"Namespace": "diaspora",
37+
"Name": "diaspora",
38+
"FullName": "diaspora/diaspora",
39+
"Perm": {
40+
"Pull": true,
41+
"Push": false,
42+
"Admin": false
43+
},
44+
"Branch": "master",
45+
"Private": false,
46+
"Clone": "https://gitlab.com/diaspora/diaspora.git",
47+
"CloneSSH": "[email protected]:diaspora/diaspora.git",
48+
"Link": "",
49+
"Created": "0001-01-01T00:00:00Z",
50+
"Updated": "0001-01-01T00:00:00Z"
3651
}
3752
},
3853
"Base": {
3954
"Ref": "master",
4055
"Repo": {
4156
"ID": "32732",
42-
"Name": "diaspora",
4357
"Namespace": "diaspora",
44-
"FullName": "diaspora/diaspora"
58+
"Name": "diaspora",
59+
"FullName": "diaspora/diaspora",
60+
"Perm": {
61+
"Pull": true,
62+
"Push": false,
63+
"Admin": false
64+
},
65+
"Branch": "master",
66+
"Private": false,
67+
"Clone": "https://gitlab.com/diaspora/diaspora.git",
68+
"CloneSSH": "[email protected]:diaspora/diaspora.git",
69+
"Link": "",
70+
"Created": "0001-01-01T00:00:00Z",
71+
"Updated": "0001-01-01T00:00:00Z"
4572
}
4673
}
4774
}

0 commit comments

Comments
 (0)