github/drone
5
0
Fork 0
mirror of https://github.com/harness/drone.git synced 2025-05-16 17:09:58 +08:00
Code Issues Packages Projects Releases Wiki Activity

[MERGE] Enhance PR Merge (Check) Fields. (#342)

Browse Source
This commit is contained in:
Johannes Batzill 2023-02-14 20:18:10 -08:00 committed by GitHub
parent 278ef7a496
commit 9d894c79cc
28 changed files with 301 additions and 235 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
gitrpc/errors.go
View File

@ -13,3 +13,4 @@ var ErrAlreadyExists = errors.New("already exists")
var ErrInvalidArgument = errors.New("invalid argument")
var ErrNotFound = errors.New("not found")
var ErrPreconditionFailed = errors.New("precondition failed")
var ErrNotMergeable = errors.New("not mergeable")

6
gitrpc/internal/gitea/merge.go
View File

@ -295,7 +295,11 @@ func (g Adapter) GetMergeBase(ctx context.Context, repoPath, remote, base, head
}
stdout, _, err := git.NewCommand(ctx, "merge-base", "--", base, head).RunStdString(&git.RunOpts{Dir: repoPath})
return strings.TrimSpace(stdout), base, err
if err != nil {
return "", "", processGiteaErrorf(err, "failed to get merge-base")
}
return strings.TrimSpace(stdout), base, nil
}
// giteaRunStdError is an implementation of the RunStdError interface in the gitea codebase.

4
gitrpc/internal/service/mapping.go
View File

@ -33,9 +33,9 @@ func processGitErrorf(err error, format string, args ...interface{}) error {
case errors.Is(err, types.ErrInvalidArgument):
return status.Errorf(codes.InvalidArgument, message)
case types.IsMergeConflictsError(err):
return status.Errorf(codes.FailedPrecondition, message)
return status.Errorf(codes.Aborted, message)
case types.IsMergeUnrelatedHistoriesError(err):
return status.Errorf(codes.FailedPrecondition, message)
return status.Errorf(codes.Aborted, message)
default:
return status.Errorf(codes.Unknown, message)
}

29
gitrpc/internal/service/merge.go
View File

@ -73,9 +73,20 @@ func (s MergeService) Merge(
// no error check needed, all branches were created when creating the temporary repo
baseBranch := "base"
trackingBranch := "tracking"
baseCommitSHA, _, _ := s.adapter.GetMergeBase(ctx, tmpBasePath, "origin", baseBranch, trackingBranch)
headCommit, _ := s.adapter.GetCommit(ctx, tmpBasePath, trackingBranch)
headCommit, err := s.adapter.GetCommit(ctx, tmpBasePath, trackingBranch)
if err != nil {
return nil, fmt.Errorf("failed to get commit of tracking branch (head): %w", err)
}
headCommitSHA := headCommit.SHA
baseCommit, err := s.adapter.GetCommit(ctx, tmpBasePath, baseBranch)
if err != nil {
return nil, fmt.Errorf("failed to get commit of base branch: %w", err)
}
baseCommitSHA := baseCommit.SHA
mergeBaseCommitSHA, _, err := s.adapter.GetMergeBase(ctx, tmpBasePath, "origin", baseBranch, trackingBranch)
if err != nil {
return nil, fmt.Errorf("failed to get merge base: %w", err)
}
if request.HeadExpectedSha != "" && request.HeadExpectedSha != headCommitSHA {
return nil, status.Errorf(
@ -170,9 +181,10 @@ func (s MergeService) Merge(
refType := enum.RefFromRPC(request.RefType)
if refType == enum.RefTypeUndefined {
return &rpc.MergeResponse{
MergeSha: mergeCommitSHA,
BaseSha: baseCommitSHA,
HeadSha: headCommitSHA,
BaseSha: baseCommitSHA,
HeadSha: headCommitSHA,
MergeBaseSha: mergeBaseCommitSHA,
MergeSha: mergeCommitSHA,
}, nil
}
@ -193,9 +205,10 @@ func (s MergeService) Merge(
}
return &rpc.MergeResponse{
MergeSha: mergeCommitSHA,
BaseSha: baseCommitSHA,
HeadSha: headCommitSHA,
BaseSha: baseCommitSHA,
HeadSha: headCommitSHA,
MergeBaseSha: mergeBaseCommitSHA,
MergeSha: mergeCommitSHA,
}, nil
}

3
gitrpc/mapping.go
View File

@ -39,6 +39,9 @@ func processRPCErrorf(err error, format string, args ...interface{}) error {
return ErrInvalidArgument
case rpcErr.Code() == codes.FailedPrecondition:
return ErrPreconditionFailed
case rpcErr.Code() == codes.Aborted:
// TODO: this should not be so generic ...
return ErrNotMergeable
default:
return fallbackErr
}

18
gitrpc/merge.go
View File

@ -41,9 +41,14 @@ type MergeParams struct {
// MergeOutput is result object from merging and returns
// base, head and commit sha.
type MergeOutput struct {
MergedSHA string
BaseSHA string
HeadSHA string
// BaseSHA is the sha of the latest commit on the base branch that was used for merging.
BaseSHA string
// HeadSHA is the sha of the latest commit on the head branch that was used for merging.
HeadSHA string
// MergeBaseSHA is the sha of the merge base of the HeadSHA and BaseSHA
MergeBaseSHA string
// MergeSHA is the sha of the commit after merging HeadSHA with BaseSHA.
MergeSHA string
}
// Merge method executes git merge operation. Refs can be sha, branch or tag.
@ -83,8 +88,9 @@ func (c *Client) Merge(ctx context.Context, params *MergeParams) (MergeOutput, e
}
return MergeOutput{
MergedSHA: resp.GetMergeSha(),
BaseSHA: resp.GetBaseSha(),
HeadSHA: resp.GetHeadSha(),
BaseSHA: resp.GetBaseSha(),
HeadSHA: resp.GetHeadSha(),
MergeBaseSHA: resp.GetMergeBaseSha(),
MergeSHA: resp.GetMergeSha(),
}, nil
}

12
gitrpc/proto/merge.proto
View File

@ -46,8 +46,12 @@ message MergeRequest {
}
message MergeResponse {
// The merge_sha is merge commit between head_sha and base_sha
string merge_sha = 1;
string base_sha = 2;
string head_sha = 3;
// base_sha is the sha of the latest commit on the base branch that was used for merging.
string base_sha = 1;
// head_sha is the sha of the latest commit on the head branch that was used for merging.
string head_sha = 2;
// merge_base_sha is the sha of the merge base of the head_sha and base_sha
string merge_base_sha = 3;
// merge_sha is the sha of the commit after merging head_sha with base_sha.
string merge_sha = 4;
}

63
gitrpc/rpc/merge.pb.go
View File

@ -175,10 +175,14 @@ type MergeResponse struct {
sizeCache protoimpl.SizeCache
unknownFields protoimpl.UnknownFields
// The merge_sha is merge commit between head_sha and base_sha
MergeSha string `protobuf:"bytes,1,opt,name=merge_sha,json=mergeSha,proto3" json:"merge_sha,omitempty"`
BaseSha string `protobuf:"bytes,2,opt,name=base_sha,json=baseSha,proto3" json:"base_sha,omitempty"`
HeadSha string `protobuf:"bytes,3,opt,name=head_sha,json=headSha,proto3" json:"head_sha,omitempty"`
// base_sha is the sha of the latest commit on the base branch that was used for merging.
BaseSha string `protobuf:"bytes,1,opt,name=base_sha,json=baseSha,proto3" json:"base_sha,omitempty"`
// head_sha is the sha of the latest commit on the head branch that was used for merging.
HeadSha string `protobuf:"bytes,2,opt,name=head_sha,json=headSha,proto3" json:"head_sha,omitempty"`
// merge_base_sha is the sha of the merge base of the head_sha and base_sha
MergeBaseSha string `protobuf:"bytes,3,opt,name=merge_base_sha,json=mergeBaseSha,proto3" json:"merge_base_sha,omitempty"`
// merge_sha is the sha of the commit after merging head_sha with base_sha.
MergeSha string `protobuf:"bytes,4,opt,name=merge_sha,json=mergeSha,proto3" json:"merge_sha,omitempty"`
}
func (x *MergeResponse) Reset() {
@ -213,13 +217,6 @@ func (*MergeResponse) Descriptor() ([]byte, []int) {
return file_merge_proto_rawDescGZIP(), []int{1}
}
func (x *MergeResponse) GetMergeSha() string {
if x != nil {
return x.MergeSha
}
return ""
}
func (x *MergeResponse) GetBaseSha() string {
if x != nil {
return x.BaseSha
@ -234,6 +231,20 @@ func (x *MergeResponse) GetHeadSha() string {
return ""
}
func (x *MergeResponse) GetMergeBaseSha() string {
if x != nil {
return x.MergeBaseSha
}
return ""
}
func (x *MergeResponse) GetMergeSha() string {
if x != nil {
return x.MergeSha
}
return ""
}
var File_merge_proto protoreflect.FileDescriptor
var file_merge_proto_rawDesc = []byte{
@ -266,20 +277,22 @@ var file_merge_proto_rawDesc = []byte{
0x6f, 0x72, 0x63, 0x65, 0x12, 0x2c, 0x0a, 0x12, 0x64, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x5f, 0x68,
0x65, 0x61, 0x64, 0x5f, 0x62, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x18, 0x0c, 0x20, 0x01, 0x28, 0x08,
0x52, 0x10, 0x64, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x48, 0x65, 0x61, 0x64, 0x42, 0x72, 0x61, 0x6e,
0x63, 0x68, 0x22, 0x62, 0x0a, 0x0d, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65, 0x73, 0x70, 0x6f,
0x6e, 0x73, 0x65, 0x12, 0x1b, 0x0a, 0x09, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x5f, 0x73, 0x68, 0x61,
0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x68, 0x61,
0x12, 0x19, 0x0a, 0x08, 0x62, 0x61, 0x73, 0x65, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x02, 0x20, 0x01,
0x28, 0x09, 0x52, 0x07, 0x62, 0x61, 0x73, 0x65, 0x53, 0x68, 0x61, 0x12, 0x19, 0x0a, 0x08, 0x68,
0x65, 0x61, 0x64, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x68,
0x65, 0x61, 0x64, 0x53, 0x68, 0x61, 0x32, 0x40, 0x0a, 0x0c, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x53,
0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x30, 0x0a, 0x05, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x12,
0x11, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65, 0x71, 0x75, 0x65,
0x73, 0x74, 0x1a, 0x12, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65,
0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42, 0x27, 0x5a, 0x25, 0x67, 0x69, 0x74, 0x68,
0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x72, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67,
0x69, 0x74, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69, 0x74, 0x72, 0x70, 0x63, 0x2f, 0x72, 0x70,
0x63, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
0x63, 0x68, 0x22, 0x88, 0x01, 0x0a, 0x0d, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65, 0x73, 0x70,
0x6f, 0x6e, 0x73, 0x65, 0x12, 0x19, 0x0a, 0x08, 0x62, 0x61, 0x73, 0x65, 0x5f, 0x73, 0x68, 0x61,
0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x62, 0x61, 0x73, 0x65, 0x53, 0x68, 0x61, 0x12,
0x19, 0x0a, 0x08, 0x68, 0x65, 0x61, 0x64, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x02, 0x20, 0x01, 0x28,
0x09, 0x52, 0x07, 0x68, 0x65, 0x61, 0x64, 0x53, 0x68, 0x61, 0x12, 0x24, 0x0a, 0x0e, 0x6d, 0x65,
0x72, 0x67, 0x65, 0x5f, 0x62, 0x61, 0x73, 0x65, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x03, 0x20, 0x01,
0x28, 0x09, 0x52, 0x0c, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x61, 0x73, 0x65, 0x53, 0x68, 0x61,
0x12, 0x1b, 0x0a, 0x09, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x5f, 0x73, 0x68, 0x61, 0x18, 0x04, 0x20,
0x01, 0x28, 0x09, 0x52, 0x08, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x68, 0x61, 0x32, 0x40, 0x0a,
0x0c, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x30, 0x0a,
0x05, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x12, 0x11, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72,
0x67, 0x65, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x12, 0x2e, 0x72, 0x70, 0x63, 0x2e,
0x4d, 0x65, 0x72, 0x67, 0x65, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x42,
0x27, 0x5a, 0x25, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61,
0x72, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69, 0x74, 0x6e, 0x65, 0x73, 0x73, 0x2f, 0x67, 0x69,
0x74, 0x72, 0x70, 0x63, 0x2f, 0x72, 0x70, 0x63, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33,
}
var (

37
internal/api/controller/pullreq/merge.go
View File

@ -23,8 +23,8 @@ import (
)
type MergeInput struct {
Method enum.MergeMethod `json:"method"`
HeadSHA string `json:"head_sha"`
Method enum.MergeMethod `json:"method"`
SourceSHA string `json:"source_sha"`
}
// Merge merges the pull request.
@ -116,23 +116,26 @@ func (c *Controller) Merge(
Author: rpcIdentityFromPrincipal(session.Principal),
RefType: gitrpcenum.RefTypeBranch,
RefName: pr.TargetBranch,
HeadExpectedSHA: in.HeadSHA,
HeadExpectedSHA: in.SourceSHA,
})
if err != nil {
return types.MergeResponse{}, err
}
pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
now := time.Now().UnixMilli()
pr.MergeStrategy = &in.Method
pr.Merged = &now
pr.MergedBy = &session.Principal.ID
pr.State = enum.PullReqStateMerged
// update all SHAs (might be empty if previous merge check failed)
pr.MergeRefSHA = &mergeOutput.MergedSHA
pr.MergeBaseSHA = &mergeOutput.BaseSHA
pr.MergeHeadSHA = &mergeOutput.HeadSHA
now := time.Now().UnixMilli()
pr.Merged = &now
pr.MergedBy = &session.Principal.ID
pr.MergeMethod = &in.Method
// update all Merge specific information (might be empty if previous merge check failed)
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeTargetSHA = &mergeOutput.BaseSHA
pr.MergeBaseSHA = &mergeOutput.MergeBaseSHA
pr.MergeSHA = &mergeOutput.MergeSHA
pr.MergeConflicts = nil
pr.ActivitySeq++ // because we need to write the activity entry
return nil
@ -143,9 +146,9 @@ func (c *Controller) Merge(
activityPayload := &types.PullRequestActivityPayloadMerge{
MergeMethod: in.Method,
MergedSHA: mergeOutput.MergedSHA,
BaseSHA: mergeOutput.BaseSHA,
HeadSHA: mergeOutput.HeadSHA,
MergeSHA: mergeOutput.MergeSHA,
TargetSHA: mergeOutput.BaseSHA,
SourceSHA: mergeOutput.HeadSHA,
}
if _, errAct := c.activityStore.CreateWithPayload(ctx, pr, session.Principal.ID, activityPayload); errAct != nil {
// non-critical error
@ -155,9 +158,9 @@ func (c *Controller) Merge(
c.eventReporter.Merged(ctx, &pullreqevents.MergedPayload{
Base: eventBase(pr, &session.Principal),
MergeMethod: in.Method,
MergedSHA: mergeOutput.MergedSHA,
BaseSHA: mergeOutput.BaseSHA,
HeadSHA: mergeOutput.HeadSHA,
MergeSHA: mergeOutput.MergeSHA,
TargetSHA: mergeOutput.BaseSHA,
SourceSHA: mergeOutput.HeadSHA,
})
return types.MergeResponse{

4
internal/api/controller/pullreq/pr_commits.go
View File

@ -34,9 +34,11 @@ func (c *Controller) Commits(
}
gitRef := pr.SourceBranch
if pr.SourceSHA != "" {
gitRef = pr.SourceSHA
}
afterRef := pr.TargetBranch
if pr.State == enum.PullReqStateMerged {
gitRef = *pr.MergeHeadSHA
afterRef = *pr.MergeBaseSHA
}

45
internal/api/controller/pullreq/pr_create.go
View File

@ -107,27 +107,28 @@ func newPullReq(
) *types.PullReq {
now := time.Now().UnixMilli()
return &types.PullReq{
ID: 0, // the ID will be populated in the data layer
Version: 0,
Number: number,
CreatedBy: session.Principal.ID,
Created: now,
Updated: now,
Edited: now,
State: enum.PullReqStateOpen,
IsDraft: in.IsDraft,
Title: in.Title,
Description: in.Description,
SourceRepoID: sourceRepo.ID,
SourceBranch: in.SourceBranch,
SourceSHA: sourceSHA,
TargetRepoID: targetRepo.ID,
TargetBranch: in.TargetBranch,
ActivitySeq: 0,
MergedBy: nil,
Merged: nil,
MergeStrategy: nil,
Author: *session.Principal.ToPrincipalInfo(),
Merger: nil,
ID: 0, // the ID will be populated in the data layer
Version: 0,
Number: number,
CreatedBy: session.Principal.ID,
Created: now,
Updated: now,
Edited: now,
State: enum.PullReqStateOpen,
IsDraft: in.IsDraft,
Title: in.Title,
Description: in.Description,
SourceRepoID: sourceRepo.ID,
SourceBranch: in.SourceBranch,
SourceSHA: sourceSHA,
TargetRepoID: targetRepo.ID,
TargetBranch: in.TargetBranch,
ActivitySeq: 0,
MergedBy: nil,
Merged: nil,
MergeCheckStatus: enum.MergeCheckStatusUnchecked,
MergeMethod: nil,
Author: *session.Principal.ToPrincipalInfo(),
Merger: nil,
}
}

10
internal/api/controller/pullreq/pr_diff.go
View File

@ -33,14 +33,12 @@ func (c *Controller) RawDiff(
}
headRef := pr.SourceBranch
if pr.SourceSHA != "" {
headRef = pr.SourceSHA
}
baseRef := pr.TargetBranch
if pr.State == enum.PullReqStateMerged {
if pr.MergeBaseSHA != nil {
baseRef = *pr.MergeBaseSHA
}
if pr.MergeHeadSHA != nil {
headRef = *pr.MergeHeadSHA
}
baseRef = *pr.MergeBaseSHA
}
return c.gitRPCClient.RawDiff(ctx, &gitrpc.DiffParams{

31
internal/api/controller/pullreq/pr_find.go
View File

@ -7,7 +7,6 @@ package pullreq
import (
"context"
"fmt"
"strings"
"github.com/harness/gitness/gitrpc"
"github.com/harness/gitness/internal/api/usererror"
@ -37,14 +36,14 @@ func (c *Controller) Find(
return nil, err
}
headRef := pr.SourceBranch
if pr.SourceSHA != "" {
headRef = pr.SourceSHA
}
baseRef := pr.TargetBranch
headRef := pr.SourceSHA
if pr.MergeBaseSHA != nil {
baseRef = *pr.MergeBaseSHA
}
if pr.MergeHeadSHA != nil {
headRef = *pr.MergeHeadSHA
}
output, err := c.gitRPCClient.DiffStats(ctx, &gitrpc.DiffParams{
ReadParams: gitrpc.CreateRPCReadParams(repo),
@ -58,27 +57,5 @@ func (c *Controller) Find(
pr.Stats.DiffStats.Commits = output.Commits
pr.Stats.DiffStats.FilesChanged = output.FilesChanged
updateMergeStatus(pr)
return pr, nil
}
func updateMergeStatus(pr *types.PullReq) {
mc := ""
if pr.MergeConflicts != nil {
mc = strings.TrimSpace(*pr.MergeConflicts)
}
switch {
case pr.State == enum.PullReqStateClosed:
pr.MergeStatus = enum.MergeStatusClosed
case pr.IsDraft:
pr.MergeStatus = enum.MergeStatusDraft
case mc != "":
pr.MergeStatus = enum.MergeStatusConflict
case pr.MergeRefSHA != nil:
pr.MergeStatus = enum.MergeStatusMergeable
default:
pr.MergeStatus = enum.MergeStatusUnchecked
}
}

4
internal/api/controller/pullreq/pr_list.go
View File

@ -64,9 +64,5 @@ func (c *Controller) List(
return nil, 0, err
}
for i := range list {
updateMergeStatus(list[i])
}
return list, count, nil
}

7
internal/api/controller/pullreq/pr_state.go
View File

@ -122,9 +122,12 @@ func (c *Controller) State(ctx context.Context,
pr.IsDraft = in.IsDraft
pr.Edited = time.Now().UnixMilli()
if in.State == enum.PullReqStateClosed {
pr.MergeRefSHA = nil
// clear all merge (check) related fields
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeTargetSHA = nil
pr.MergeBaseSHA = nil
pr.MergeHeadSHA = nil
pr.MergeSHA = nil
pr.MergeConflicts = nil
}
pr.ActivitySeq++ // because we need to add the activity entry
return nil

2
internal/api/controller/repo/merge_check.go
View File

@ -50,7 +50,7 @@ func (c *Controller) MergeCheck(
HeadRepoUID: writeParams.RepoUID, // forks are not supported for now
HeadBranch: info.HeadRef,
})
if errors.Is(err, gitrpc.ErrPreconditionFailed) {
if errors.Is(err, gitrpc.ErrNotMergeable) {
return MergeCheck{
Mergeable: false,
}, nil

2
internal/api/usererror/translate.go
View File

@ -60,6 +60,8 @@ func Translate(err error) *Error {
return ErrNotFound
case errors.Is(err, gitrpc.ErrPreconditionFailed):
return ErrPreconditionFailed
case errors.Is(err, gitrpc.ErrNotMergeable):
return ErrNotMergeable
// webhook errors
case errors.Is(err, webhook.ErrWebhookNotRetriggerable):

3
internal/api/usererror/usererror.go
View File

@ -31,6 +31,9 @@ var (
// ErrPreconditionFailed is returned when a precondition failed.
ErrPreconditionFailed = New(http.StatusPreconditionFailed, "Precondition failed")
// ErrNotMergeable is returned when a branch can't be merged.
ErrNotMergeable = New(http.StatusPreconditionFailed, "Branch can't be merged")
// ErrNoChange is returned when no change was found based on the request.
ErrNoChange = New(http.StatusBadRequest, "No Change")

6
internal/events/pullreq/events_state.go
View File

@ -97,9 +97,9 @@ const MergedEvent events.EventType = "merged"
type MergedPayload struct {
Base
MergeMethod enum.MergeMethod `json:"merge_method"`
MergedSHA string `json:"merged_sha"`
BaseSHA string `json:"base_sha"`
HeadSHA string `json:"head_sha"`
MergeSHA string `json:"merge_sha"`
TargetSHA string `json:"target_sha"`
SourceSHA string `json:"source_sha"`
}
func (r *Reporter) Merged(ctx context.Context, payload *MergedPayload) {

21
internal/services/pullreq/handlers_branch.go
View File

@ -31,10 +31,15 @@ func (s *Service) triggerPREventOnBranchUpdate(ctx context.Context,
"failed to set SourceSHA for PR %d to value '%s', expected SHA '%s' but current pr has '%s'",
pr.Number, event.Payload.NewSHA, event.Payload.OldSHA, pr.SourceSHA)
}
pr.SourceSHA = event.Payload.NewSHA
pr.MergeRefSHA = nil
// reset merge-check fields for new run
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeTargetSHA = nil
pr.MergeBaseSHA = nil
pr.MergeHeadSHA = nil
pr.MergeSHA = nil
pr.MergeConflicts = nil
return nil
})
if err != nil {
@ -76,11 +81,15 @@ func (s *Service) closePullReqOnBranchDelete(ctx context.Context,
) error {
s.forEveryOpenPR(ctx, event.Payload.RepoID, event.Payload.Ref, func(pr *types.PullReq) error {
pr, err := s.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
pr.State = enum.PullReqStateClosed
pr.MergeRefSHA = nil
pr.MergeBaseSHA = nil
pr.MergeHeadSHA = nil
pr.ActivitySeq++ // because we need to write the activity
pr.State = enum.PullReqStateClosed
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeTargetSHA = nil
pr.MergeBaseSHA = nil
pr.MergeSHA = nil
pr.MergeConflicts = nil
return nil
})
if err != nil {

27
internal/services/pullreq/handlers_mergeable.go
View File

@ -207,23 +207,36 @@ func (s *Service) updateMergeData(
Force: true,
})
if errors.Is(err, gitrpc.ErrPreconditionFailed) {
// TODO: in case of merge conflict, update conflicts in pr entry - for that we need a
// nice way to distinguish between the two errors.
return events.NewDiscardEventErrorf("Source branch '%s' is not on SHA '%s' anymore or is not mergeable.",
return events.NewDiscardEventErrorf("Source branch '%s' is not on SHA '%s' anymore.",
pr.SourceBranch, newSHA)
}
if err != nil {
isNotMergeableError := errors.Is(err, gitrpc.ErrNotMergeable)
if err != nil && !isNotMergeableError {
return fmt.Errorf("merge check failed for %s and %s with err: %w",
targetRepo.UID+":"+pr.TargetBranch, sourceRepo.UID+":"+pr.SourceBranch, err)
}
// Update DB in both cases (failure or success)
_, err = s.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
if pr.SourceSHA != newSHA {
return events.NewDiscardEventErrorf("PR SHA %s is newer than %s", pr.SourceSHA, newSHA)
}
pr.MergeRefSHA = &output.MergedSHA
pr.MergeBaseSHA = &output.BaseSHA
pr.MergeHeadSHA = &output.HeadSHA
if isNotMergeableError {
// TODO: gitrpc should return sha's either way, and also conflicting files!
pr.MergeCheckStatus = enum.MergeCheckStatusConflict
pr.MergeTargetSHA = nil
pr.MergeBaseSHA = nil
pr.MergeSHA = nil
pr.MergeConflicts = nil
} else {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeTargetSHA = &output.BaseSHA
pr.MergeBaseSHA = &output.MergeBaseSHA
pr.MergeSHA = &output.MergeSHA
pr.MergeConflicts = nil
}
return nil
})
if err != nil {

2
internal/services/webhook/types.go
View File

@ -99,7 +99,7 @@ func pullReqInfoFrom(pr *types.PullReq) PullReqInfo {
SourceBranch: pr.SourceBranch,
TargetRepoID: pr.TargetRepoID,
TargetBranch: pr.TargetBranch,
MergeStrategy: pr.MergeStrategy,
MergeStrategy: pr.MergeMethod,
}
}

7
internal/store/database/migrate/postgres/0003_create_table_pullreqs.up.sql
View File

@ -19,10 +19,11 @@ pullreq_id SERIAL PRIMARY KEY
,pullreq_activity_seq INTEGER DEFAULT 0
,pullreq_merged_by INTEGER
,pullreq_merged BIGINT
,pullreq_merge_strategy TEXT
,pullreq_merge_head_sha TEXT
,pullreq_merge_method TEXT
,pullreq_merge_check_status TEXT
,pullreq_merge_target_sha TEXT
,pullreq_merge_base_sha TEXT
,pullreq_merge_ref_sha TEXT
,pullreq_merge_sha TEXT
,pullreq_merge_conflicts TEXT
,CONSTRAINT fk_pullreq_created_by FOREIGN KEY (pullreq_created_by)
REFERENCES principals (principal_id) MATCH SIMPLE

7
internal/store/database/migrate/sqlite/0003_create_table_pullreqs.up.sql
View File

@ -19,10 +19,11 @@ pullreq_id INTEGER PRIMARY KEY AUTOINCREMENT
,pullreq_activity_seq INTEGER DEFAULT 0
,pullreq_merged_by INTEGER
,pullreq_merged BIGINT
,pullreq_merge_strategy TEXT
,pullreq_merge_head_sha TEXT
,pullreq_merge_method TEXT
,pullreq_merge_check_status TEXT NOT NULL
,pullreq_merge_target_sha TEXT
,pullreq_merge_base_sha TEXT
,pullreq_merge_ref_sha TEXT
,pullreq_merge_sha TEXT
,pullreq_merge_conflicts TEXT
,CONSTRAINT fk_pullreq_created_by FOREIGN KEY (pullreq_created_by)
REFERENCES principals (principal_id) MATCH SIMPLE

150
internal/store/database/pullreq.go
View File

@ -67,13 +67,15 @@ type pullReq struct {
ActivitySeq int64 `db:"pullreq_activity_seq"`
MergedBy null.Int `db:"pullreq_merged_by"`
Merged null.Int `db:"pullreq_merged"`
MergeStrategy null.String `db:"pullreq_merge_strategy"`
MergeHeadSHA null.String `db:"pullreq_merge_head_sha"`
MergeBaseSHA null.String `db:"pullreq_merge_base_sha"`
MergeRefSHA null.String `db:"pullreq_merge_ref_sha"`
MergeConflicts null.String `db:"pullreq_merge_conflicts"`
MergedBy null.Int `db:"pullreq_merged_by"`
Merged null.Int `db:"pullreq_merged"`
MergeMethod null.String `db:"pullreq_merge_method"`
MergeCheckStatus enum.MergeCheckStatus `db:"pullreq_merge_check_status"`
MergeTargetSHA null.String `db:"pullreq_merge_target_sha"`
MergeBaseSHA null.String `db:"pullreq_merge_base_sha"`
MergeSHA null.String `db:"pullreq_merge_sha"`
MergeConflicts null.String `db:"pullreq_merge_conflicts"`
}
const (
@ -98,10 +100,11 @@ const (
,pullreq_activity_seq
,pullreq_merged_by
,pullreq_merged
,pullreq_merge_strategy
,pullreq_merge_head_sha
,pullreq_merge_method
,pullreq_merge_check_status
,pullreq_merge_target_sha
,pullreq_merge_base_sha
,pullreq_merge_ref_sha
,pullreq_merge_sha
,pullreq_merge_conflicts`
pullReqSelectBase = `
@ -185,8 +188,11 @@ func (s *PullReqStore) Create(ctx context.Context, pr *types.PullReq) error {
,pullreq_activity_seq
,pullreq_merged_by
,pullreq_merged
,pullreq_merge_strategy
,pullreq_merge_ref_sha
,pullreq_merge_method
,pullreq_merge_check_status
,pullreq_merge_target_sha
,pullreq_merge_base_sha
,pullreq_merge_sha
,pullreq_merge_conflicts
) values (
:pullreq_version
@ -208,8 +214,11 @@ func (s *PullReqStore) Create(ctx context.Context, pr *types.PullReq) error {
,:pullreq_activity_seq
,:pullreq_merged_by
,:pullreq_merged
,:pullreq_merge_strategy
,:pullreq_merge_ref_sha
,:pullreq_merge_method
,:pullreq_merge_check_status
,:pullreq_merge_target_sha
,:pullreq_merge_base_sha
,:pullreq_merge_sha
,:pullreq_merge_conflicts
) RETURNING pullreq_id`
@ -244,10 +253,11 @@ func (s *PullReqStore) Update(ctx context.Context, pr *types.PullReq) error {
,pullreq_source_sha = :pullreq_source_sha
,pullreq_merged_by = :pullreq_merged_by
,pullreq_merged = :pullreq_merged
,pullreq_merge_strategy = :pullreq_merge_strategy
,pullreq_merge_head_sha = :pullreq_merge_head_sha
,pullreq_merge_method = :pullreq_merge_method
,pullreq_merge_check_status = :pullreq_merge_check_status
,pullreq_merge_target_sha = :pullreq_merge_target_sha
,pullreq_merge_base_sha = :pullreq_merge_base_sha
,pullreq_merge_ref_sha = :pullreq_merge_ref_sha
,pullreq_merge_sha = :pullreq_merge_sha
,pullreq_merge_conflicts = :pullreq_merge_conflicts
WHERE pullreq_id = :pullreq_id AND pullreq_version = :pullreq_version - 1`
@ -448,33 +458,34 @@ func (s *PullReqStore) List(ctx context.Context, opts *types.PullReqFilter) ([]*
func mapPullReq(pr *pullReq) *types.PullReq {
return &types.PullReq{
ID: pr.ID,
Version: pr.Version,
Number: pr.Number,
CreatedBy: pr.CreatedBy,
Created: pr.Created,
Updated: pr.Updated,
Edited: pr.Edited,
State: pr.State,
IsDraft: pr.IsDraft,
CommentCount: pr.CommentCount,
Title: pr.Title,
Description: pr.Description,
SourceRepoID: pr.SourceRepoID,
SourceBranch: pr.SourceBranch,
SourceSHA: pr.SourceSHA,
TargetRepoID: pr.TargetRepoID,
TargetBranch: pr.TargetBranch,
ActivitySeq: pr.ActivitySeq,
MergedBy: pr.MergedBy.Ptr(),
Merged: pr.Merged.Ptr(),
MergeStrategy: (*enum.MergeMethod)(pr.MergeStrategy.Ptr()),
MergeHeadSHA: pr.MergeHeadSHA.Ptr(),
MergeBaseSHA: pr.MergeBaseSHA.Ptr(),
MergeRefSHA: pr.MergeRefSHA.Ptr(),
MergeConflicts: pr.MergeConflicts.Ptr(),
Author: types.PrincipalInfo{},
Merger: nil,
ID: pr.ID,
Version: pr.Version,
Number: pr.Number,
CreatedBy: pr.CreatedBy,
Created: pr.Created,
Updated: pr.Updated,
Edited: pr.Edited,
State: pr.State,
IsDraft: pr.IsDraft,
CommentCount: pr.CommentCount,
Title: pr.Title,
Description: pr.Description,
SourceRepoID: pr.SourceRepoID,
SourceBranch: pr.SourceBranch,
SourceSHA: pr.SourceSHA,
TargetRepoID: pr.TargetRepoID,
TargetBranch: pr.TargetBranch,
ActivitySeq: pr.ActivitySeq,
MergedBy: pr.MergedBy.Ptr(),
Merged: pr.Merged.Ptr(),
MergeMethod: (*enum.MergeMethod)(pr.MergeMethod.Ptr()),
MergeCheckStatus: pr.MergeCheckStatus,
MergeTargetSHA: pr.MergeTargetSHA.Ptr(),
MergeBaseSHA: pr.MergeBaseSHA.Ptr(),
MergeSHA: pr.MergeSHA.Ptr(),
MergeConflicts: pr.MergeConflicts.Ptr(),
Author: types.PrincipalInfo{},
Merger: nil,
Stats: types.PullReqStats{
Conversations: pr.CommentCount,
DiffStats: types.DiffStats{
@ -487,31 +498,32 @@ func mapPullReq(pr *pullReq) *types.PullReq {
func mapInternalPullReq(pr *types.PullReq) *pullReq {
m := &pullReq{
ID: pr.ID,
Version: pr.Version,
Number: pr.Number,
CreatedBy: pr.CreatedBy,
Created: pr.Created,
Updated: pr.Updated,
Edited: pr.Edited,
State: pr.State,
IsDraft: pr.IsDraft,
CommentCount: pr.CommentCount,
Title: pr.Title,
Description: pr.Description,
SourceRepoID: pr.SourceRepoID,
SourceBranch: pr.SourceBranch,
SourceSHA: pr.SourceSHA,
TargetRepoID: pr.TargetRepoID,
TargetBranch: pr.TargetBranch,
ActivitySeq: pr.ActivitySeq,
MergedBy: null.IntFromPtr(pr.MergedBy),
Merged: null.IntFromPtr(pr.Merged),
MergeStrategy: null.StringFromPtr((*string)(pr.MergeStrategy)),
MergeHeadSHA: null.StringFromPtr(pr.MergeHeadSHA),
MergeBaseSHA: null.StringFromPtr(pr.MergeBaseSHA),
MergeRefSHA: null.StringFromPtr(pr.MergeRefSHA),
MergeConflicts: null.StringFromPtr(pr.MergeConflicts),
ID: pr.ID,
Version: pr.Version,
Number: pr.Number,
CreatedBy: pr.CreatedBy,
Created: pr.Created,
Updated: pr.Updated,
Edited: pr.Edited,
State: pr.State,
IsDraft: pr.IsDraft,
CommentCount: pr.CommentCount,
Title: pr.Title,
Description: pr.Description,
SourceRepoID: pr.SourceRepoID,
SourceBranch: pr.SourceBranch,
SourceSHA: pr.SourceSHA,
TargetRepoID: pr.TargetRepoID,
TargetBranch: pr.TargetBranch,
ActivitySeq: pr.ActivitySeq,
MergedBy: null.IntFromPtr(pr.MergedBy),
Merged: null.IntFromPtr(pr.Merged),
MergeMethod: null.StringFromPtr((*string)(pr.MergeMethod)),
MergeCheckStatus: pr.MergeCheckStatus,
MergeTargetSHA: null.StringFromPtr(pr.MergeTargetSHA),
MergeBaseSHA: null.StringFromPtr(pr.MergeBaseSHA),
MergeSHA: null.StringFromPtr(pr.MergeSHA),
MergeConflicts: null.StringFromPtr(pr.MergeConflicts),
}
return m

13
types/enum/pullreq.go
View File

@ -189,12 +189,13 @@ var mergeMethods = sortEnum([]MergeMethod{
MergeMethodRebase,
})
type MergeStatus string
type MergeCheckStatus string
const (
MergeStatusUnchecked MergeStatus = "unchecked" // The merge status has not been checked.
MergeStatusConflict MergeStatus = "conflict" // Can’t merge into the target branch due to a potential conflict.
MergeStatusDraft MergeStatus = "draft_pr" // Can’t merge because the pull request is a draft.
MergeStatusClosed MergeStatus = "closed" // The merge request must be open before merge.
MergeStatusMergeable MergeStatus = "mergeable" // The branch can merge cleanly into the target branch.
// MergeCheckStatusUnchecked merge status has not been checked.
MergeCheckStatusUnchecked MergeCheckStatus = "unchecked"
// MergeCheckStatusConflict can’t merge into the target branch due to a potential conflict.
MergeCheckStatusConflict MergeCheckStatus = "conflict"
// MergeCheckStatusMergeable branch can merged cleanly into the target branch.
MergeCheckStatusMergeable MergeCheckStatus = "mergeable"
)

16
types/pullreq.go
View File

@ -35,15 +35,15 @@ type PullReq struct {
ActivitySeq int64 `json:"-"` // not returned, because it's a server's internal field
MergedBy *int64 `json:"-"` // not returned, because the merger info is in the Merger field
Merged *int64 `json:"merged"`
MergeStrategy *enum.MergeMethod `json:"merge_strategy"`
MergedBy *int64 `json:"-"` // not returned, because the merger info is in the Merger field
Merged *int64 `json:"merged"`
MergeMethod *enum.MergeMethod `json:"merge_method"`
MergeHeadSHA *string `json:"merge_head_sha"`
MergeBaseSHA *string `json:"merge_base_sha"`
MergeRefSHA *string `json:"merge_ref_sha"`
MergeStatus enum.MergeStatus `json:"merge_status"`
MergeConflicts *string `json:"merge_conflicts,omitempty"`
MergeCheckStatus enum.MergeCheckStatus `json:"merge_check_status"`
MergeTargetSHA *string `json:"merge_target_sha"`
MergeBaseSHA *string `json:"merge_base_sha"`
MergeSHA *string `json:"merge_sha"`
MergeConflicts *string `json:"merge_conflicts,omitempty"`
Author PrincipalInfo `json:"author"`
Merger *PrincipalInfo `json:"merger"`

6
types/pullreq_activity.go
View File

@ -166,9 +166,9 @@ func (a *PullRequestActivityPayloadComment) ActivityType() enum.PullReqActivityT
type PullRequestActivityPayloadMerge struct {
MergeMethod enum.MergeMethod `json:"merge_method"`
MergedSHA string `json:"merged_sha"`
BaseSHA string `json:"base_sha"`
HeadSHA string `json:"head_sha"`
MergeSHA string `json:"merge_sha"`
TargetSHA string `json:"target_sha"`
SourceSHA string `json:"source_sha"`
}
func (a *PullRequestActivityPayloadMerge) ActivityType() enum.PullReqActivityType {