Skip to content

Commit 52f7ca5

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 92f2d90 commit 52f7ca5

File tree

4 files changed

+31
-43
lines changed

4 files changed

+31
-43
lines changed

‎modules/structs/pull.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ type PullRequest struct {
2727
Comments int `json:"comments"`
2828
// number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR)
2929
ReviewComments int `json:"review_comments"`
30-
Additions int `json:"additions"`
31-
Deletions int `json:"deletions"`
32-
ChangedFiles int `json:"changed_files"`
30+
31+
Additions *int `json:"additions,omitempty"`
32+
Deletions *int `json:"deletions,omitempty"`
33+
ChangedFiles *int `json:"changed_files,omitempty"`
3334

3435
HTMLURL string `json:"html_url"`
3536
DiffURL string `json:"diff_url"`

‎services/convert/pull.go

+3-17
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,11 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u
239239
// Calculate diff
240240
startCommitID = pr.MergeBase
241241

242-
apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID)
242+
diffChangedFiles, diffAdditions, diffDeletions, err := gitRepo.GetDiffShortStat(startCommitID, endCommitID)
243243
if err != nil {
244244
log.Error("GetDiffShortStat: %v", err)
245+
} else {
246+
apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions = &diffChangedFiles, &diffAdditions, &diffDeletions
245247
}
246248
}
247249

@@ -459,12 +461,6 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
459461
return nil, err
460462
}
461463

462-
// Outer scope variables to be used in diff calculation
463-
var (
464-
startCommitID string
465-
endCommitID string
466-
)
467-
468464
if git.IsErrBranchNotExist(err) {
469465
headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref)
470466
if err != nil && !git.IsErrNotExist(err) {
@@ -473,7 +469,6 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
473469
}
474470
if err == nil {
475471
apiPullRequest.Head.Sha = headCommitID
476-
endCommitID = headCommitID
477472
}
478473
} else {
479474
commit, err := headBranch.GetCommit()
@@ -484,17 +479,8 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
484479
if err == nil {
485480
apiPullRequest.Head.Ref = pr.HeadBranch
486481
apiPullRequest.Head.Sha = commit.ID.String()
487-
endCommitID = commit.ID.String()
488482
}
489483
}
490-
491-
// Calculate diff
492-
startCommitID = pr.MergeBase
493-
494-
apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID)
495-
if err != nil {
496-
log.Error("GetDiffShortStat: %v", err)
497-
}
498484
}
499485

500486
if len(apiPullRequest.Head.Sha) == 0 && len(apiPullRequest.Head.Ref) != 0 {

‎tests/integration/api_pull_test.go

+19-21
Original file line numberDiff line numberDiff line change
@@ -51,30 +51,30 @@ func TestAPIViewPulls(t *testing.T) {
5151
assert.Empty(t, pull.RequestedReviewersTeams)
5252
assert.EqualValues(t, 5, pull.RequestedReviewers[0].ID)
5353
assert.EqualValues(t, 6, pull.RequestedReviewers[1].ID)
54-
assert.EqualValues(t, 1, pull.ChangedFiles)
5554

5655
if assert.EqualValues(t, 5, pull.ID) {
5756
resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK)
5857
bs, err := io.ReadAll(resp.Body)
5958
assert.NoError(t, err)
6059
patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "")
6160
assert.NoError(t, err)
62-
if assert.Len(t, patch.Files, pull.ChangedFiles) {
61+
if assert.Len(t, patch.Files, 1) {
6362
assert.Equal(t, "File-WoW", patch.Files[0].Name)
6463
// FIXME: The old name should be empty if it's a file add type
6564
assert.Equal(t, "File-WoW", patch.Files[0].OldName)
66-
assert.EqualValues(t, pull.Additions, patch.Files[0].Addition)
67-
assert.EqualValues(t, pull.Deletions, patch.Files[0].Deletion)
65+
assert.EqualValues(t, 1, patch.Files[0].Addition)
66+
assert.EqualValues(t, 0, patch.Files[0].Deletion)
6867
assert.Equal(t, gitdiff.DiffFileAdd, patch.Files[0].Type)
6968
}
7069

7170
t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID),
7271
doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) {
73-
if assert.Len(t, files, pull.ChangedFiles) {
72+
if assert.Len(t, files, 1) {
7473
assert.Equal(t, "File-WoW", files[0].Filename)
7574
assert.Empty(t, files[0].PreviousFilename)
76-
assert.EqualValues(t, pull.Additions, files[0].Additions)
77-
assert.EqualValues(t, pull.Deletions, files[0].Deletions)
75+
assert.EqualValues(t, 1, files[0].Additions)
76+
assert.EqualValues(t, 1, files[0].Changes)
77+
assert.EqualValues(t, 0, files[0].Deletions)
7878
assert.Equal(t, "added", files[0].Status)
7979
}
8080
}))
@@ -88,53 +88,51 @@ func TestAPIViewPulls(t *testing.T) {
8888
assert.EqualValues(t, 4, pull.RequestedReviewers[1].ID)
8989
assert.EqualValues(t, 2, pull.RequestedReviewers[2].ID)
9090
assert.EqualValues(t, 5, pull.RequestedReviewers[3].ID)
91-
assert.EqualValues(t, 1, pull.ChangedFiles)
9291

9392
if assert.EqualValues(t, 2, pull.ID) {
9493
resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK)
9594
bs, err := io.ReadAll(resp.Body)
9695
assert.NoError(t, err)
9796
patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "")
9897
assert.NoError(t, err)
99-
if assert.Len(t, patch.Files, pull.ChangedFiles) {
98+
if assert.Len(t, patch.Files, 1) {
10099
assert.Equal(t, "README.md", patch.Files[0].Name)
101100
assert.Equal(t, "README.md", patch.Files[0].OldName)
102-
assert.EqualValues(t, pull.Additions, patch.Files[0].Addition)
103-
assert.EqualValues(t, pull.Deletions, patch.Files[0].Deletion)
101+
assert.EqualValues(t, 4, patch.Files[0].Addition)
102+
assert.EqualValues(t, 1, patch.Files[0].Deletion)
104103
assert.Equal(t, gitdiff.DiffFileChange, patch.Files[0].Type)
105104
}
106105

107106
t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID),
108107
doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) {
109-
if assert.Len(t, files, pull.ChangedFiles) {
108+
if assert.Len(t, files, 1) {
110109
assert.Equal(t, "README.md", files[0].Filename)
111110
// FIXME: The PreviousFilename name should be the same as Filename if it's a file change
112111
assert.Equal(t, "", files[0].PreviousFilename)
113-
assert.EqualValues(t, pull.Additions, files[0].Additions)
114-
assert.EqualValues(t, pull.Deletions, files[0].Deletions)
112+
assert.EqualValues(t, 4, files[0].Additions)
113+
assert.EqualValues(t, 1, files[0].Deletions)
115114
assert.Equal(t, "changed", files[0].Status)
116115
}
117116
}))
118117
}
119118

120-
pull = pulls[2]
119+
pull = pulls[0]
121120
assert.EqualValues(t, 1, pull.Poster.ID)
122-
assert.Len(t, pull.RequestedReviewers, 1)
121+
assert.Len(t, pull.RequestedReviewers, 2)
123122
assert.Empty(t, pull.RequestedReviewersTeams)
124-
assert.EqualValues(t, 1, pull.RequestedReviewers[0].ID)
125-
assert.EqualValues(t, 0, pull.ChangedFiles)
123+
assert.EqualValues(t, 5, pull.RequestedReviewers[0].ID)
126124

127-
if assert.EqualValues(t, 1, pull.ID) {
125+
if assert.EqualValues(t, 5, pull.ID) {
128126
resp = ctx.Session.MakeRequest(t, NewRequest(t, "GET", pull.DiffURL), http.StatusOK)
129127
bs, err := io.ReadAll(resp.Body)
130128
assert.NoError(t, err)
131129
patch, err := gitdiff.ParsePatch(context.Background(), 1000, 5000, 10, bytes.NewReader(bs), "")
132130
assert.NoError(t, err)
133-
assert.EqualValues(t, pull.ChangedFiles, patch.NumFiles)
131+
assert.EqualValues(t, 1, patch.NumFiles)
134132

135133
t.Run(fmt.Sprintf("APIGetPullFiles_%d", pull.ID),
136134
doAPIGetPullFiles(ctx, pull, func(t *testing.T, files []*api.ChangedFile) {
137-
assert.Len(t, files, pull.ChangedFiles)
135+
assert.Len(t, files, 1)
138136
}))
139137
}
140138
}

‎tests/integration/repo_webhook_test.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
"github.com/PuerkitoBio/goquery"
2727
"github.com/stretchr/testify/assert"
28+
"github.com/stretchr/testify/require"
2829
)
2930

3031
func TestNewWebHookLink(t *testing.T) {
@@ -378,12 +379,14 @@ func Test_WebhookPullRequest(t *testing.T) {
378379

379380
// 3. validate the webhook is triggered
380381
assert.EqualValues(t, "pull_request", triggeredEvent)
381-
assert.Len(t, payloads, 1)
382+
require.Len(t, payloads, 1)
382383
assert.EqualValues(t, "repo1", payloads[0].PullRequest.Base.Repository.Name)
383384
assert.EqualValues(t, "user2/repo1", payloads[0].PullRequest.Base.Repository.FullName)
384385
assert.EqualValues(t, "repo1", payloads[0].PullRequest.Head.Repository.Name)
385386
assert.EqualValues(t, "user2/repo1", payloads[0].PullRequest.Head.Repository.FullName)
386-
assert.EqualValues(t, 0, payloads[0].PullRequest.Additions)
387+
assert.EqualValues(t, 0, *payloads[0].PullRequest.Additions)
388+
assert.EqualValues(t, 0, *payloads[0].PullRequest.ChangedFiles)
389+
assert.EqualValues(t, 0, *payloads[0].PullRequest.Deletions)
387390
})
388391
}
389392

0 commit comments

Comments
 (0)