From 1013da463f4c84c55ba4f5672c916c0763aa89da Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 5 Jun 2024 15:17:11 +0200 Subject: [PATCH 1/3] test: webhook open project expected fields --- services/webhook/default_test.go | 36 ++++++++++++++++++++++++++++++++ services/webhook/general_test.go | 11 ++++++++++ 2 files changed, 47 insertions(+) diff --git a/services/webhook/default_test.go b/services/webhook/default_test.go index 29f1521cca..e6e59fed03 100644 --- a/services/webhook/default_test.go +++ b/services/webhook/default_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/json" webhook_module "code.gitea.io/gitea/modules/webhook" + jsoniter "github.com/json-iterator/go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -222,3 +223,38 @@ func TestForgejoPayload(t *testing.T) { assert.Equal(t, "refs/heads/test", body.Ref) // full ref }) } + +func TestOpenProjectPayload(t *testing.T) { + t.Run("PullRequest", func(t *testing.T) { + p := pullRequestTestPayload() + data, err := p.JSONPayload() + require.NoError(t, err) + + // adapted from https://github.com/opf/openproject/blob/4c5c45fe995da0060902bc8dd5f1bf704d0b8737/modules/github_integration/lib/open_project/github_integration/services/upsert_pull_request.rb#L56 + j := jsoniter.Get(data, "pull_request") + + assert.Equal(t, 12, j.Get("id").MustBeValid().ToInt()) + assert.Equal(t, "user1", j.Get("user", "login").MustBeValid().ToString()) + assert.Equal(t, 12, j.Get("number").MustBeValid().ToInt()) + assert.Equal(t, "http://localhost:3000/test/repo/pulls/12", j.Get("html_url").MustBeValid().ToString()) + assert.Equal(t, jsoniter.NilValue, j.Get("updated_at").ValueType()) + assert.Equal(t, "", j.Get("state").MustBeValid().ToString()) + assert.Equal(t, "Fix bug", j.Get("title").MustBeValid().ToString()) + assert.Equal(t, "fixes bug #2", j.Get("body").MustBeValid().ToString()) + + assert.Equal(t, "test/repo", j.Get("base", "repo", "full_name").MustBeValid().ToString()) + assert.Equal(t, "http://localhost:3000/test/repo", j.Get("base", "repo", "html_url").MustBeValid().ToString()) + + assert.Equal(t, false, j.Get("draft").MustBeValid().ToBool()) + assert.Equal(t, jsoniter.NilValue, j.Get("merge_commit_sha").ValueType()) + assert.Equal(t, false, j.Get("merged").MustBeValid().ToBool()) + assert.Equal(t, jsoniter.NilValue, j.Get("merged_by").ValueType()) + assert.Equal(t, jsoniter.NilValue, j.Get("merged_at").ValueType()) + assert.Equal(t, 0, j.Get("comments").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("review_comments").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("additions").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("deletions").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("changed_files").MustBeValid().ToInt()) + // assert.Equal(t,"labels:", j.Get("labels").map { |values| extract_label_values(values) ) + }) +} diff --git a/services/webhook/general_test.go b/services/webhook/general_test.go index 41bac3fd04..5b9bfdc1b2 100644 --- a/services/webhook/general_test.go +++ b/services/webhook/general_test.go @@ -281,6 +281,17 @@ func pullRequestTestPayload() *api.PullRequestPayload { Title: "Milestone Title", Description: "Milestone Description", }, + Base: &api.PRBranchInfo{ + Name: "branch1", + Ref: "refs/pull/2/head", + Sha: "4a357436d925b5c974181ff12a994538ddc5a269", + RepoID: 1, + Repository: &api.Repository{ + HTMLURL: "http://localhost:3000/test/repo", + Name: "repo", + FullName: "test/repo", + }, + }, }, Review: &api.ReviewPayload{ Content: "good job", From fb7b17d24081535212e7e06e6fcfcaa6327dbc82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Rosenhammer?= Date: Sun, 26 May 2024 06:08:13 +0200 Subject: [PATCH 2/3] Make gitea webhooks openproject compatible (gitea#28435) This PR adds some fields to the gitea webhook payload that [openproject](https://www.openproject.org/) expects to exists in order to process the webhooks. These fields do exists in Github's webhook payload so adding them makes Gitea's native webhook more compatible towards Github's. --- models/issues/pull.go | 15 ++++++++ modules/structs/issue.go | 1 + modules/structs/pull.go | 6 ++++ modules/structs/user.go | 2 ++ services/convert/issue.go | 2 ++ services/convert/pull.go | 64 ++++++++++++++++++++++------------ services/convert/user.go | 1 + templates/swagger/v1_json.tmpl | 34 ++++++++++++++++++ 8 files changed, 102 insertions(+), 23 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index ae7fb35eab..ef49a51045 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -431,6 +431,21 @@ func (pr *PullRequest) GetGitHeadBranchRefName() string { return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch) } +// GetReviewCommentsCount returns the number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR) +func (pr *PullRequest) GetReviewCommentsCount(ctx context.Context) int { + opts := FindCommentsOptions{ + Type: CommentTypeReview, + IssueID: pr.IssueID, + } + conds := opts.ToConds() + + count, err := db.GetEngine(ctx).Where(conds).Count(new(Comment)) + if err != nil { + return 0 + } + return int(count) +} + // IsChecking returns true if this pull request is still checking conflict. func (pr *PullRequest) IsChecking() bool { return pr.Status == PullRequestStatusChecking diff --git a/modules/structs/issue.go b/modules/structs/issue.go index e2b49e94c5..7ba7f77158 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -30,6 +30,7 @@ type PullRequestMeta struct { HasMerged bool `json:"merged"` Merged *time.Time `json:"merged_at"` IsWorkInProgress bool `json:"draft"` + HTMLURL string `json:"html_url"` } // RepositoryMeta basic repository information diff --git a/modules/structs/pull.go b/modules/structs/pull.go index b04def52b8..525d90c28e 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -21,8 +21,14 @@ type PullRequest struct { Assignees []*User `json:"assignees"` RequestedReviewers []*User `json:"requested_reviewers"` State StateType `json:"state"` + Draft bool `json:"draft"` IsLocked bool `json:"is_locked"` Comments int `json:"comments"` + // number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR) + ReviewComments int `json:"review_comments"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + ChangedFiles int `json:"changed_files"` HTMLURL string `json:"html_url"` DiffURL string `json:"diff_url"` diff --git a/modules/structs/user.go b/modules/structs/user.go index ad529c966e..be20349e53 100644 --- a/modules/structs/user.go +++ b/modules/structs/user.go @@ -27,6 +27,8 @@ type User struct { Email string `json:"email"` // URL to the user's avatar AvatarURL string `json:"avatar_url"` + // URL to the user's gitea page + HTMLURL string `json:"html_url"` // User locale Language string `json:"language"` // Is the user an administrator diff --git a/services/convert/issue.go b/services/convert/issue.go index 047feb8aa7..f514dc4313 100644 --- a/services/convert/issue.go +++ b/services/convert/issue.go @@ -107,6 +107,8 @@ func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Iss if issue.PullRequest.HasMerged { apiIssue.PullRequest.Merged = issue.PullRequest.MergedUnix.AsTimePtr() } + // Add pr's html url + apiIssue.PullRequest.HTMLURL = issue.HTMLURL() } } if issue.DeadlineUnix != 0 { diff --git a/services/convert/pull.go b/services/convert/pull.go index da75cb3db2..c214805ed5 100644 --- a/services/convert/pull.go +++ b/services/convert/pull.go @@ -61,29 +61,31 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } apiPullRequest := &api.PullRequest{ - ID: pr.ID, - URL: pr.Issue.HTMLURL(), - Index: pr.Index, - Poster: apiIssue.Poster, - Title: apiIssue.Title, - Body: apiIssue.Body, - Labels: apiIssue.Labels, - Milestone: apiIssue.Milestone, - Assignee: apiIssue.Assignee, - Assignees: apiIssue.Assignees, - State: apiIssue.State, - IsLocked: apiIssue.IsLocked, - Comments: apiIssue.Comments, - HTMLURL: pr.Issue.HTMLURL(), - DiffURL: pr.Issue.DiffURL(), - PatchURL: pr.Issue.PatchURL(), - HasMerged: pr.HasMerged, - MergeBase: pr.MergeBase, - Mergeable: pr.Mergeable(ctx), - Deadline: apiIssue.Deadline, - Created: pr.Issue.CreatedUnix.AsTimePtr(), - Updated: pr.Issue.UpdatedUnix.AsTimePtr(), - PinOrder: apiIssue.PinOrder, + ID: pr.ID, + URL: pr.Issue.HTMLURL(), + Index: pr.Index, + Poster: apiIssue.Poster, + Title: apiIssue.Title, + Body: apiIssue.Body, + Labels: apiIssue.Labels, + Milestone: apiIssue.Milestone, + Assignee: apiIssue.Assignee, + Assignees: apiIssue.Assignees, + State: apiIssue.State, + Draft: pr.IsWorkInProgress(ctx), + IsLocked: apiIssue.IsLocked, + Comments: apiIssue.Comments, + ReviewComments: pr.GetReviewCommentsCount(ctx), + HTMLURL: pr.Issue.HTMLURL(), + DiffURL: pr.Issue.DiffURL(), + PatchURL: pr.Issue.PatchURL(), + HasMerged: pr.HasMerged, + MergeBase: pr.MergeBase, + Mergeable: pr.Mergeable(ctx), + Deadline: apiIssue.Deadline, + Created: pr.Issue.CreatedUnix.AsTimePtr(), + Updated: pr.Issue.UpdatedUnix.AsTimePtr(), + PinOrder: apiIssue.PinOrder, AllowMaintainerEdit: pr.AllowMaintainerEdit, @@ -178,6 +180,12 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u return nil } + // Outer scope variables to be used in diff calculation + var ( + startCommitID string + endCommitID string + ) + if git.IsErrBranchNotExist(err) { headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref) if err != nil && !git.IsErrNotExist(err) { @@ -186,6 +194,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } if err == nil { apiPullRequest.Head.Sha = headCommitID + endCommitID = headCommitID } } else { commit, err := headBranch.GetCommit() @@ -196,8 +205,17 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u if err == nil { apiPullRequest.Head.Ref = pr.HeadBranch apiPullRequest.Head.Sha = commit.ID.String() + endCommitID = commit.ID.String() } } + + // Calculate diff + startCommitID = pr.MergeBase + + apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID) + if err != nil { + log.Error("GetDiffShortStat: %v", err) + } } if len(apiPullRequest.Head.Sha) == 0 && len(apiPullRequest.Head.Ref) != 0 { diff --git a/services/convert/user.go b/services/convert/user.go index 789bc51097..94a400de5d 100644 --- a/services/convert/user.go +++ b/services/convert/user.go @@ -53,6 +53,7 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap FullName: user.FullName, Email: user.GetPlaceholderEmail(), AvatarURL: user.AvatarLink(ctx), + HTMLURL: user.HTMLURL(), Created: user.CreatedUnix.AsTime(), Restricted: user.IsRestricted, Location: user.Location, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6a488559a0..8e9ee8b5cd 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -23413,6 +23413,11 @@ "description": "PullRequest represents a pull request", "type": "object", "properties": { + "additions": { + "type": "integer", + "format": "int64", + "x-go-name": "Additions" + }, "allow_maintainer_edit": { "type": "boolean", "x-go-name": "AllowMaintainerEdit" @@ -23434,6 +23439,11 @@ "type": "string", "x-go-name": "Body" }, + "changed_files": { + "type": "integer", + "format": "int64", + "x-go-name": "ChangedFiles" + }, "closed_at": { "type": "string", "format": "date-time", @@ -23449,10 +23459,19 @@ "format": "date-time", "x-go-name": "Created" }, + "deletions": { + "type": "integer", + "format": "int64", + "x-go-name": "Deletions" + }, "diff_url": { "type": "string", "x-go-name": "DiffURL" }, + "draft": { + "type": "boolean", + "x-go-name": "Draft" + }, "due_date": { "type": "string", "format": "date-time", @@ -23529,6 +23548,12 @@ }, "x-go-name": "RequestedReviewers" }, + "review_comments": { + "description": "number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR)", + "type": "integer", + "format": "int64", + "x-go-name": "ReviewComments" + }, "state": { "$ref": "#/definitions/StateType" }, @@ -23559,6 +23584,10 @@ "type": "boolean", "x-go-name": "IsWorkInProgress" }, + "html_url": { + "type": "string", + "x-go-name": "HTMLURL" + }, "merged": { "type": "boolean", "x-go-name": "HasMerged" @@ -24904,6 +24933,11 @@ "type": "string", "x-go-name": "FullName" }, + "html_url": { + "description": "URL to the user's gitea page", + "type": "string", + "x-go-name": "HTMLURL" + }, "id": { "description": "the user's id", "type": "integer", From 8763225972945a42b0a08ff29c3e9e3ddbae4955 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 5 Jun 2024 15:21:57 +0200 Subject: [PATCH 3/3] add release note --- release-notes/8.0.0/feat/4027.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 release-notes/8.0.0/feat/4027.md diff --git a/release-notes/8.0.0/feat/4027.md b/release-notes/8.0.0/feat/4027.md new file mode 100644 index 0000000000..bdae695679 --- /dev/null +++ b/release-notes/8.0.0/feat/4027.md @@ -0,0 +1 @@ +- Gitea/Forgejo webhook payload include additional fields (`html_url`, `additions`, `deletions`, `review_comments`...) for better compatbility with [OpenProject](https://www.openproject.org/), ported from [gitea#28435](https://github.com/go-gitea/gitea/pull/28435).