[MISC] Webhook Creation Default To Secure, Fix PR/Compare Commit Listing, Fix Merge Commit Message (#203)

This change is fixing a few things on both UI and backend side:
- update webhook creation screen to default to enable ssl verification (secure by default should always be the move)
- fix pr/compare commit listing to show correct diffs the head branch is ahead of the base branch (NOTE: THIS ONLY SHOWS THE LATEST 20 COMMITS - NO PAGINATION ADDED)
- fix merge commit message to contain the correct head branch, source repo path and pr numbers (similar'ish to github)
jobatzil/rename
Johannes Batzill 2023-01-13 02:49:39 -08:00 committed by GitHub
parent d9a01ef9a3
commit c827fa5e66
20 changed files with 100 additions and 71 deletions

View File

@ -176,10 +176,26 @@ func (g Adapter) Merge(
mergeMethod string,
trackingBranch string,
tmpBasePath string,
mergeMsg string,
env []string,
) error {
// TODO: mergeMethod should be an enum.
if mergeMethod != "merge" {
return fmt.Errorf("merge method '%s' is not supported", mergeMethod)
}
var outbuf, errbuf strings.Builder
cmd := git.NewCommand(ctx, "merge", "--no-ff", trackingBranch)
args := []string{
mergeMethod,
"--no-ff",
trackingBranch,
}
// override message for merging iff mergeMsg was provided (only for merge for now)
if mergeMethod == "merge" && mergeMsg != "" {
args = append(args, "-m", mergeMsg)
}
cmd := git.NewCommand(ctx, args...)
if err := cmd.Run(&git.RunOpts{
Env: env,
Dir: tmpBasePath,

View File

@ -44,6 +44,6 @@ type GitAdapter interface {
GetRef(ctx context.Context, repoPath string, name string, refType types.RefType) (string, error)
CreateTemporaryRepoForPR(ctx context.Context, reposTempPath string, pr *types.PullRequest) (string, error)
Merge(ctx context.Context, pr *types.PullRequest, mergeMethod string, trackingBranch string,
tmpBasePath string, env []string) error
tmpBasePath string, mergeMsg string, env []string) error
GetDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) (string, error)
}

View File

@ -49,7 +49,7 @@ func (s MergeService) MergeBranch(
repoPath := getFullPathForRepo(s.reposRoot, request.GetBase().GetRepoUid())
pr := &types.PullRequest{
BaseRepoPath: repoPath,
BaseBranch: request.GetBranch(),
BaseBranch: request.GetBaseBranch(),
HeadBranch: request.GetHeadBranch(),
}
// Clone base repo.
@ -127,7 +127,12 @@ func (s MergeService) MergeBranch(
"GIT_COMMITTER_DATE="+commitTimeStr,
)
if err = s.adapter.Merge(ctx, pr, "merge", trackingBranch, tmpBasePath, env); err != nil {
mergeMsg := strings.TrimSpace(request.GetTitle())
if len(request.GetMessage()) > 0 {
mergeMsg += "\n\n" + strings.TrimSpace(request.GetMessage())
}
if err = s.adapter.Merge(ctx, pr, "merge", trackingBranch, tmpBasePath, mergeMsg, env); err != nil {
return nil, err
}
@ -169,7 +174,7 @@ func validateMergeBranchRequest(request *rpc.MergeBranchRequest) error {
return fmt.Errorf("empty user name")
}
if len(request.Branch) == 0 {
if len(request.BaseBranch) == 0 {
return fmt.Errorf("empty branch name")
}

View File

@ -122,7 +122,7 @@ func (s *CommitFilesService) CommitFiles(stream rpc.CommitFilesService_CommitFil
message := strings.TrimSpace(header.GetTitle())
if len(header.GetMessage()) > 0 {
message += "\n\n" + header.GetMessage()
message += "\n\n" + strings.TrimSpace(header.GetMessage())
}
// Now commit the tree
commitHash, err := shared.CommitTree(ctx, commit.ID.String(), author, committer, treeHash, message, false)

View File

@ -12,11 +12,13 @@ import (
type MergeBranchParams struct {
WriteParams
BaseBranch string
HeadRepoUID string
HeadBranch string
Force bool
DeleteBranch bool
BaseBranch string
HeadRepoUID string
HeadBranch string
Title string
Message string
Force bool
DeleteHeadBranch bool
}
func (c *Client) MergeBranch(ctx context.Context, params *MergeBranchParams) (string, error) {
@ -25,11 +27,13 @@ func (c *Client) MergeBranch(ctx context.Context, params *MergeBranchParams) (st
}
resp, err := c.mergeService.MergeBranch(ctx, &rpc.MergeBranchRequest{
Base: mapToRPCWriteRequest(params.WriteParams),
Branch: params.BaseBranch,
HeadBranch: params.HeadBranch,
Force: params.Force,
Delete: params.DeleteBranch,
Base: mapToRPCWriteRequest(params.WriteParams),
BaseBranch: params.BaseBranch,
HeadBranch: params.HeadBranch,
Title: params.Title,
Message: params.Message,
Force: params.Force,
DeleteHeadBranch: params.DeleteHeadBranch,
})
if err != nil {
return "", err

View File

@ -16,9 +16,9 @@ message MergeBranchRequest {
WriteRequest base = 1;
// head_branch is the source branch we want to merge
string head_branch = 2;
// branch is the branch into which the given commit shall be merged and whose
// base_branch is the branch into which the given commit shall be merged and whose
// reference is going to be updated.
string branch = 3;
string base_branch = 3;
// title is the title to use for the merge commit.
string title = 4;
// message is the message to use for the merge commit.
@ -26,7 +26,7 @@ message MergeBranchRequest {
// force merge
bool force = 6;
// delete branch after merge
bool delete = 7;
bool delete_head_branch = 7;
}
// This comment is left unintentionally blank.

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// protoc-gen-go v1.28.1
// protoc v3.21.9
// protoc v3.21.11
// source: diff.proto
package rpc

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
// versions:
// - protoc-gen-go-grpc v1.2.0
// - protoc v3.21.9
// - protoc v3.21.11
// source: diff.proto
package rpc

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// protoc-gen-go v1.28.1
// protoc v3.21.9
// protoc v3.21.11
// source: http.proto
package rpc
@ -151,7 +151,6 @@ type ServicePackRequest struct {
// Depending on the service the matching base type has to be passed
//
// Types that are assignable to Base:
//
// *ServicePackRequest_ReadBase
// *ServicePackRequest_WriteBase
Base isServicePackRequest_Base `protobuf_oneof:"base"`

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
// versions:
// - protoc-gen-go-grpc v1.2.0
// - protoc v3.21.9
// - protoc v3.21.11
// source: http.proto
package rpc

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// protoc-gen-go v1.28.1
// protoc v3.21.9
// protoc v3.21.11
// source: merge.proto
package rpc
@ -28,9 +28,9 @@ type MergeBranchRequest struct {
Base *WriteRequest `protobuf:"bytes,1,opt,name=base,proto3" json:"base,omitempty"`
// head_branch is the source branch we want to merge
HeadBranch string `protobuf:"bytes,2,opt,name=head_branch,json=headBranch,proto3" json:"head_branch,omitempty"`
// branch is the branch into which the given commit shall be merged and whose
// base_branch is the branch into which the given commit shall be merged and whose
// reference is going to be updated.
Branch string `protobuf:"bytes,3,opt,name=branch,proto3" json:"branch,omitempty"`
BaseBranch string `protobuf:"bytes,3,opt,name=base_branch,json=baseBranch,proto3" json:"base_branch,omitempty"`
// title is the title to use for the merge commit.
Title string `protobuf:"bytes,4,opt,name=title,proto3" json:"title,omitempty"`
// message is the message to use for the merge commit.
@ -38,7 +38,7 @@ type MergeBranchRequest struct {
// force merge
Force bool `protobuf:"varint,6,opt,name=force,proto3" json:"force,omitempty"`
// delete branch after merge
Delete bool `protobuf:"varint,7,opt,name=delete,proto3" json:"delete,omitempty"`
DeleteHeadBranch bool `protobuf:"varint,7,opt,name=delete_head_branch,json=deleteHeadBranch,proto3" json:"delete_head_branch,omitempty"`
}
func (x *MergeBranchRequest) Reset() {
@ -87,9 +87,9 @@ func (x *MergeBranchRequest) GetHeadBranch() string {
return ""
}
func (x *MergeBranchRequest) GetBranch() string {
func (x *MergeBranchRequest) GetBaseBranch() string {
if x != nil {
return x.Branch
return x.BaseBranch
}
return ""
}
@ -115,9 +115,9 @@ func (x *MergeBranchRequest) GetForce() bool {
return false
}
func (x *MergeBranchRequest) GetDelete() bool {
func (x *MergeBranchRequest) GetDeleteHeadBranch() bool {
if x != nil {
return x.Delete
return x.DeleteHeadBranch
}
return false
}
@ -176,31 +176,33 @@ var File_merge_proto protoreflect.FileDescriptor
var file_merge_proto_rawDesc = []byte{
0x0a, 0x0b, 0x6d, 0x65, 0x72, 0x67, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x03, 0x72,
0x70, 0x63, 0x1a, 0x0c, 0x73, 0x68, 0x61, 0x72, 0x65, 0x64, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f,
0x22, 0xd2, 0x01, 0x0a, 0x12, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68,
0x22, 0xf1, 0x01, 0x0a, 0x12, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68,
0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x25, 0x0a, 0x04, 0x62, 0x61, 0x73, 0x65, 0x18,
0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x11, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x57, 0x72, 0x69, 0x74,
0x65, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x52, 0x04, 0x62, 0x61, 0x73, 0x65, 0x12, 0x1f,
0x0a, 0x0b, 0x68, 0x65, 0x61, 0x64, 0x5f, 0x62, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x18, 0x02, 0x20,
0x01, 0x28, 0x09, 0x52, 0x0a, 0x68, 0x65, 0x61, 0x64, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x12,
0x16, 0x0a, 0x06, 0x62, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52,
0x06, 0x62, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x12, 0x14, 0x0a, 0x05, 0x74, 0x69, 0x74, 0x6c, 0x65,
0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x74, 0x69, 0x74, 0x6c, 0x65, 0x12, 0x18, 0x0a,
0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x05, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07,
0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x66, 0x6f, 0x72, 0x63, 0x65,
0x18, 0x06, 0x20, 0x01, 0x28, 0x08, 0x52, 0x05, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x12, 0x16, 0x0a,
0x06, 0x64, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x18, 0x07, 0x20, 0x01, 0x28, 0x08, 0x52, 0x06, 0x64,
0x65, 0x6c, 0x65, 0x74, 0x65, 0x22, 0x32, 0x0a, 0x13, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72,
0x61, 0x6e, 0x63, 0x68, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x1b, 0x0a, 0x09,
0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52,
0x08, 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x49, 0x64, 0x32, 0x52, 0x0a, 0x0c, 0x4d, 0x65, 0x72,
0x67, 0x65, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x42, 0x0a, 0x0b, 0x4d, 0x65, 0x72,
0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x12, 0x17, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d,
0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73,
0x74, 0x1a, 0x18, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61,
0x6e, 0x63, 0x68, 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,
0x1f, 0x0a, 0x0b, 0x62, 0x61, 0x73, 0x65, 0x5f, 0x62, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x18, 0x03,
0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x62, 0x61, 0x73, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68,
0x12, 0x14, 0x0a, 0x05, 0x74, 0x69, 0x74, 0x6c, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52,
0x05, 0x74, 0x69, 0x74, 0x6c, 0x65, 0x12, 0x18, 0x0a, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67,
0x65, 0x18, 0x05, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65,
0x12, 0x14, 0x0a, 0x05, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x18, 0x06, 0x20, 0x01, 0x28, 0x08, 0x52,
0x05, 0x66, 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, 0x07, 0x20, 0x01,
0x28, 0x08, 0x52, 0x10, 0x64, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x48, 0x65, 0x61, 0x64, 0x42, 0x72,
0x61, 0x6e, 0x63, 0x68, 0x22, 0x32, 0x0a, 0x13, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61,
0x6e, 0x63, 0x68, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x1b, 0x0a, 0x09, 0x63,
0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08,
0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x49, 0x64, 0x32, 0x52, 0x0a, 0x0c, 0x4d, 0x65, 0x72, 0x67,
0x65, 0x53, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x12, 0x42, 0x0a, 0x0b, 0x4d, 0x65, 0x72, 0x67,
0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x12, 0x17, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65,
0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e, 0x63, 0x68, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74,
0x1a, 0x18, 0x2e, 0x72, 0x70, 0x63, 0x2e, 0x4d, 0x65, 0x72, 0x67, 0x65, 0x42, 0x72, 0x61, 0x6e,
0x63, 0x68, 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 (

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
// versions:
// - protoc-gen-go-grpc v1.2.0
// - protoc v3.21.9
// - protoc v3.21.11
// source: merge.proto
package rpc

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// protoc-gen-go v1.28.1
// protoc v3.21.9
// protoc v3.21.11
// source: operations.proto
package rpc
@ -239,7 +239,6 @@ type CommitFilesAction struct {
unknownFields protoimpl.UnknownFields
// Types that are assignable to Payload:
//
// *CommitFilesAction_Header
// *CommitFilesAction_Content
Payload isCommitFilesAction_Payload `protobuf_oneof:"payload"`
@ -323,7 +322,6 @@ type CommitFilesRequest struct {
unknownFields protoimpl.UnknownFields
// Types that are assignable to Payload:
//
// *CommitFilesRequest_Header
// *CommitFilesRequest_Action
Payload isCommitFilesRequest_Payload `protobuf_oneof:"payload"`

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
// versions:
// - protoc-gen-go-grpc v1.2.0
// - protoc v3.21.9
// - protoc v3.21.11
// source: operations.proto
package rpc

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// protoc-gen-go v1.28.1
// protoc v3.21.9
// protoc v3.21.11
// source: ref.proto
package rpc

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
// versions:
// - protoc-gen-go-grpc v1.2.0
// - protoc v3.21.9
// - protoc v3.21.11
// source: ref.proto
package rpc

View File

@ -1,7 +1,7 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// protoc-gen-go v1.28.1
// protoc v3.21.9
// protoc v3.21.11
// source: shared.proto
package rpc
@ -240,7 +240,6 @@ type FileUpload struct {
unknownFields protoimpl.UnknownFields
// Types that are assignable to Data:
//
// *FileUpload_Header
// *FileUpload_Chunk
Data isFileUpload_Data `protobuf_oneof:"data"`

View File

@ -29,7 +29,7 @@ type MergeInput struct {
// Merge merges the pull request.
//
//nolint:gocognit // no need to refactor
//nolint:gocognit,funlen // no need to refactor
func (c *Controller) Merge(
ctx context.Context,
session *auth.Session,
@ -92,13 +92,19 @@ func (c *Controller) Merge(
return fmt.Errorf("failed to create RPC write params: %w", err)
}
// TODO: for forking merge title might be different?
mergeTitle := fmt.Sprintf("Merge branch '%s' of %s (#%d)", pr.SourceBranch, sourceRepo.Path, pr.Number)
// TODO: do we really want to do this in the DB transaction?
sha, err = c.gitRPCClient.MergeBranch(ctx, &gitrpc.MergeBranchParams{
WriteParams: writeParams,
BaseBranch: pr.TargetBranch,
HeadRepoUID: sourceRepo.GitUID,
HeadBranch: pr.SourceBranch,
Force: in.Force,
DeleteBranch: in.DeleteBranch,
WriteParams: writeParams,
BaseBranch: pr.TargetBranch,
HeadRepoUID: sourceRepo.GitUID,
HeadBranch: pr.SourceBranch,
Title: mergeTitle,
Message: "",
Force: in.Force,
DeleteHeadBranch: in.DeleteBranch,
})
if err != nil {
return err

View File

@ -22,7 +22,6 @@ export default function Compare() {
const { repoMetadata, error, loading, diffRefs } = useGetRepositoryMetadata()
const [sourceGitRef, setSourceGitRef] = useState(diffRefs.sourceGitRef)
const [targetGitRef, setTargetGitRef] = useState(diffRefs.targetGitRef)
const [after] = useState('')
const {
data: commits,
error: commitsError,
@ -34,7 +33,7 @@ export default function Compare() {
limit: LIST_FETCHING_LIMIT,
page: 1,
git_ref: sourceGitRef,
after
after: targetGitRef
},
lazy: !repoMetadata
})

View File

@ -17,7 +17,8 @@ export const PullRequestCommits: React.FC<Pick<GitInfoProps, 'repoMetadata' | 'p
} = useGet<RepoCommit[]>({
path: `/api/v1/repos/${repoMetadata?.path}/+/commits`,
queryParams: {
git_ref: pullRequestMetadata.source_branch
git_ref: pullRequestMetadata.source_branch,
after: pullRequestMetadata.target_branch
},
lazy: !repoMetadata
})