Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make gitea webhooks openproject compatible #28435

Merged
merged 12 commits into from
May 26, 2024
Merged

Make gitea webhooks openproject compatible #28435

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6b2157c
Add missing fields to gitea webhook for openproject compatibility.
Chief-Detektor Dec 11, 2023
99487dd
Update swagger files.
Chief-Detektor Dec 12, 2023
f0c74fb
Fix diff calculation when using sqlite.
Chief-Detektor Dec 13, 2023
635d411
Remove superfluous parameter for DiffOptions.
Chief-Detektor Dec 21, 2023
b1fe6e5
Remove suoerfluous parameter for FindCommentsOptions.
Chief-Detektor Dec 22, 2023
7b7fa4f
Use GetDiffShortStat to obtain diff changes. Add descriptive comments.
Chief-Detektor Jan 5, 2024
8e50d3f
Update swagger file to include review_comments description.
Chief-Detektor Jan 6, 2024
27312d0
Merge branch 'main' into main
wxiaoguang May 15, 2024
2632a49
Update services/convert/pull.go
6543 May 15, 2024
886337e
Merge branch 'main' into main
6543 May 15, 2024
b3b694d
Update models/issues/pull.go
wxiaoguang May 26, 2024
1e2399e
Merge branch 'main' into main
wxiaoguang May 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions models/issues/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,24 @@ 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 {
Chief-Detektor marked this conversation as resolved.
Show resolved Hide resolved
opts := FindCommentsOptions{
Chief-Detektor marked this conversation as resolved.
Show resolved Hide resolved
Type: CommentTypeReview,
IssueID: pr.IssueID,
}
conds := opts.ToConds()
if pr.ID == 0 {
conds = conds.And(builder.Eq{"invalidated": false})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks counter-intuitive. Why pr.ID == 0 means invalidated==false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I can't answer that question as I basically copied this function from here

gitea/models/issues/review.go

Line 995 in ea8e4ba

func (r *Review) GetCodeCommentsCount(ctx context.Context) int {

It could very well be that the entire check there is redundant and not needed for the function to work properly.
However, in order to evaluate that I need to revive my dev setup for this project. I'm a bit busy right now so it might take some days.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@6543 the code GetCodeCommentsCount was written in API: Add pull review endpoints (#11224)

Do you have ideas about why checking pr.ID == 0 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @6543

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @6543 ~~~

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it doesn't need this if, so let's remove it until there is a clear requirement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wxiaoguang it had it's reason back then i guess but I cant find one now

wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down
1 change: 1 addition & 0 deletions modules/structs/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
type PullRequestMeta struct {
HasMerged bool `json:"merged"`
Merged *time.Time `json:"merged_at"`
HTMLURL string `json:"html_url"`
}

// RepositoryMeta basic repository information
Expand Down
6 changes: 6 additions & 0 deletions modules/structs/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
2 changes: 2 additions & 0 deletions modules/structs/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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
Expand Down
2 changes: 2 additions & 0 deletions services/convert/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ func toIssue(ctx context.Context, issue *issues_model.Issue, getDownloadURL func
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 {
Expand Down
65 changes: 42 additions & 23 deletions services/convert/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,29 +50,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,

Expand Down Expand Up @@ -167,6 +169,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) {
Expand All @@ -175,6 +183,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()
Expand All @@ -185,8 +194,18 @@ 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)
return nil
6543 marked this conversation as resolved.
Show resolved Hide resolved
}
}

if len(apiPullRequest.Head.Sha) == 0 && len(apiPullRequest.Head.Ref) != 0 {
Expand Down
1 change: 1 addition & 0 deletions services/convert/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
34 changes: 34 additions & 0 deletions templates/swagger/v1_json.tmpl
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.