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.

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

Change target branch for pull request #6488

Merged
merged 51 commits into from
Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
f985e67
Adds functionality to change target branch of created pull requests
saitho Mar 27, 2019
1c01294
Merge remote-tracking branch 'origin/master' into feature/choose_pull…
saitho Apr 20, 2019
a520c5e
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Aug 13, 2019
6c48735
Use const instead of var in JavaScript additions
saitho Aug 13, 2019
be71ada
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Sep 17, 2019
411f54e
Check if branches are equal and if PR already exists before changing …
saitho Sep 18, 2019
4c8b956
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Sep 18, 2019
b2f8777
Make sure to check all commits
saitho Sep 18, 2019
2c02f4b
Print error messages for user as error flash message
saitho Sep 18, 2019
06adcfd
Disallow changing target branch of closed or merged pull requests
saitho Sep 18, 2019
7454709
Merge branch 'master' into feature/choose_pullreq_target
saitho Nov 20, 2019
bf60f41
Resolve conflicts after merge of upstream/master
saitho Nov 20, 2019
feade60
Change order of branch select fields
saitho Nov 20, 2019
ade1de9
Removes duplicate check
saitho Nov 24, 2019
1094cc0
Use ctx.Tr for translations
saitho Nov 24, 2019
425f631
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Nov 24, 2019
7fa44f3
Recompile JS
saitho Nov 24, 2019
139b7be
Use correct translation namespace
saitho Nov 24, 2019
5d1091b
Remove redundant if condition
saitho Nov 24, 2019
927c82a
Moves most change branch logic into pull service
saitho Nov 25, 2019
8473385
Completes comment
saitho Nov 25, 2019
0a559df
Add Ref to ChangesPayload for logging changed target branches
saitho Nov 25, 2019
234dc2a
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Nov 25, 2019
4104ce6
Revert changes to go.mod
saitho Nov 25, 2019
c2c54b7
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Dec 5, 2019
93ce415
Directly use createComment method
saitho Dec 5, 2019
a040294
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Dec 7, 2019
1566045
Return 404 if pull request is not found. Move written check up
saitho Dec 7, 2019
4c2341a
Remove variable declaration
saitho Dec 9, 2019
8007148
Return client errors on change pull request target errors
saitho Dec 9, 2019
e2d5d0d
Return error in commit.HasPreviousCommit
saitho Dec 12, 2019
2666224
Adds blank line
saitho Dec 12, 2019
c25503a
Test patch before persisting new target branch
saitho Dec 12, 2019
b9c3368
Update patch before testing (not working)
saitho Dec 12, 2019
1dcf6b6
Removes patch calls when changeing pull request target
saitho Dec 14, 2019
6a81a97
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Dec 14, 2019
7db6fc5
Removes unneeded check for base name
saitho Dec 14, 2019
1860253
Moves ChangeTargetBranch completely to pull service. Update patch sta…
saitho Dec 14, 2019
21e1c0b
Set webhook mode after errors were validated
saitho Dec 14, 2019
79347b0
Update PR in one transaction
saitho Dec 14, 2019
7ff8949
Move logic for check if head is equal with branch to pull model
saitho Dec 14, 2019
3a30a60
Adds missing comment and simplify return
saitho Dec 14, 2019
750ba50
Merge branch 'master' into feature/choose_pullreq_target
lunny Dec 14, 2019
69a2926
Merge branch 'master' into feature/choose_pullreq_target
lunny Dec 15, 2019
4e775c6
Merge branch 'master' into feature/choose_pullreq_target
zeripath Dec 15, 2019
cc39ac1
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Dec 15, 2019
ded3b68
Merge branch 'master' into feature/choose_pullreq_target
zeripath Dec 15, 2019
10cb3a5
Merge branch 'master' into feature/choose_pullreq_target
sapk Dec 15, 2019
9c04491
Merge branch 'master' into feature/choose_pullreq_target
lunny Dec 16, 2019
66de122
Merge remote-tracking branch 'upstream/master' into feature/choose_pu…
saitho Dec 16, 2019
23bd8ac
Adjust CreateComment method call
saitho Dec 16, 2019
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
55 changes: 55 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,22 @@ func (err ErrBranchNameConflict) Error() string {
return fmt.Sprintf("branch conflicts with existing branch [name: %s]", err.BranchName)
}

// ErrBranchesEqual represents an error that branch name conflicts with other branch.
type ErrBranchesEqual struct {
BaseBranchName string
HeadBranchName string
}

// IsErrBranchesEqual checks if an error is an ErrBranchesEqual.
func IsErrBranchesEqual(err error) bool {
_, ok := err.(ErrBranchesEqual)
return ok
}

func (err ErrBranchesEqual) Error() string {
return fmt.Sprintf("branches are equal [head: %sm base: %s]", err.HeadBranchName, err.BaseBranchName)
}

// ErrNotAllowedToMerge represents an error that a branch is protected and the current user is not allowed to modify it.
type ErrNotAllowedToMerge struct {
Reason string
Expand Down Expand Up @@ -1090,6 +1106,23 @@ func (err ErrIssueNotExist) Error() string {
return fmt.Sprintf("issue does not exist [id: %d, repo_id: %d, index: %d]", err.ID, err.RepoID, err.Index)
}

// ErrIssueIsClosed represents a "IssueIsClosed" kind of error.
type ErrIssueIsClosed struct {
ID int64
RepoID int64
Index int64
}

// IsErrIssueIsClosed checks if an error is a ErrIssueNotExist.
func IsErrIssueIsClosed(err error) bool {
_, ok := err.(ErrIssueIsClosed)
return ok
}

func (err ErrIssueIsClosed) Error() string {
return fmt.Sprintf("issue is closed [id: %d, repo_id: %d, index: %d]", err.ID, err.RepoID, err.Index)
}

// ErrIssueLabelTemplateLoad represents a "ErrIssueLabelTemplateLoad" kind of error.
type ErrIssueLabelTemplateLoad struct {
TemplateFile string
Expand Down Expand Up @@ -1326,6 +1359,28 @@ func (err ErrRebaseConflicts) Error() string {
return fmt.Sprintf("Rebase Error: %v: Whilst Rebasing: %s\n%s\n%s", err.Err, err.CommitSHA, err.StdErr, err.StdOut)
}

// ErrPullRequestHasMerged represents a "PullRequestHasMerged"-error
type ErrPullRequestHasMerged struct {
ID int64
IssueID int64
HeadRepoID int64
BaseRepoID int64
HeadBranch string
BaseBranch string
}

// IsErrPullRequestHasMerged checks if an error is a ErrPullRequestHasMerged.
func IsErrPullRequestHasMerged(err error) bool {
_, ok := err.(ErrPullRequestHasMerged)
return ok
}

// Error does pretty-printing :D
func (err ErrPullRequestHasMerged) Error() string {
return fmt.Sprintf("pull request has merged [id: %d, issue_id: %d, head_repo_id: %d, base_repo_id: %d, head_branch: %s, base_branch: %s]",
err.ID, err.IssueID, err.HeadRepoID, err.BaseRepoID, err.HeadBranch, err.BaseBranch)
}

// _________ __
// \_ ___ \ ____ _____ _____ ____ _____/ |_
// / \ \/ / _ \ / \ / \_/ __ \ / \ __\
Expand Down
8 changes: 8 additions & 0 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ const (
CommentTypeLock
// Unlocks a previously locked issue
CommentTypeUnlock
// Change pull request's target branch
CommentTypeChangeTargetBranch
)

// CommentTag defines comment tag type
Expand Down Expand Up @@ -116,6 +118,8 @@ type Comment struct {
Assignee *User `xorm:"-"`
OldTitle string
NewTitle string
OldRef string
NewRef string
DependentIssueID int64
DependentIssue *Issue `xorm:"-"`

Expand Down Expand Up @@ -517,6 +521,8 @@ func createCommentWithNoAction(e *xorm.Session, opts *CreateCommentOptions) (_ *
Content: opts.Content,
OldTitle: opts.OldTitle,
NewTitle: opts.NewTitle,
OldRef: opts.OldRef,
NewRef: opts.NewRef,
DependentIssueID: opts.DependentIssueID,
TreePath: opts.TreePath,
ReviewID: opts.ReviewID,
Expand Down Expand Up @@ -722,6 +728,8 @@ type CreateCommentOptions struct {
RemovedAssignee bool
OldTitle string
NewTitle string
OldRef string
NewRef string
CommitID int64
CommitSHA string
Patch string
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ var migrations = []Migration{
NewMigration("update branch protection for can push and whitelist enable", addBranchProtectionCanPushAndEnableWhitelist),
// v112 -> v113
NewMigration("remove release attachments which repository deleted", removeAttachmentMissedRepo),
// v113 -> v114
NewMigration("new feature: change target branch of pull requests", featureChangeTargetBranch),
}

// Migrate database to current version
Expand Down
23 changes: 23 additions & 0 deletions models/migrations/v113.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"fmt"

"xorm.io/xorm"
)

func featureChangeTargetBranch(x *xorm.Engine) error {
type Comment struct {
OldRef string
NewRef string
}

if err := x.Sync2(new(Comment)); err != nil {
return fmt.Errorf("Sync2: %v", err)
}
return nil
}
29 changes: 29 additions & 0 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,3 +664,32 @@ func (pr *PullRequest) GetWorkInProgressPrefix() string {
}
return ""
}

// IsHeadEqualWithBranch returns if the commits of branchName are available in pull request head
func (pr *PullRequest) IsHeadEqualWithBranch(branchName string) (bool, error) {
var err error
if err = pr.GetBaseRepo(); err != nil {
return false, err
}
baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
return false, err
}
baseCommit, err := baseGitRepo.GetBranchCommit(branchName)
if err != nil {
return false, err
}

if err = pr.GetHeadRepo(); err != nil {
return false, err
}
headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
return false, err
}
headCommit, err := headGitRepo.GetBranchCommit(pr.HeadBranch)
if err != nil {
return false, err
}
return baseCommit.HasPreviousCommit(headCommit.ID)
}
21 changes: 21 additions & 0 deletions modules/git/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,27 @@ func (c *Commit) CommitsBefore() (*list.List, error) {
return c.repo.getCommitsBefore(c.ID)
}

// HasPreviousCommit returns true if a given commitHash is contained in commit's parents
func (c *Commit) HasPreviousCommit(commitHash SHA1) (bool, error) {
for i := 0; i < c.ParentCount(); i++ {
commit, err := c.Parent(i)
if err != nil {
saitho marked this conversation as resolved.
Show resolved Hide resolved
return false, err
}
if commit.ID == commitHash {
return true, nil
}
commitInParentCommit, err := commit.HasPreviousCommit(commitHash)
if err != nil {
return false, err
}
if commitInParentCommit {
return true, nil
}
}
return false, nil
}

// CommitsBeforeLimit returns num commits before current revision
func (c *Commit) CommitsBeforeLimit(num int) (*list.List, error) {
return c.repo.getCommitsBeforeLimit(c.ID, num)
Expand Down
1 change: 1 addition & 0 deletions modules/notification/base/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type Notifier interface {
NotifyMergePullRequest(*models.PullRequest, *models.User, *git.Repository)
NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest)
NotifyPullRequestReview(*models.PullRequest, *models.Review, *models.Comment)
NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string)

NotifyCreateIssueComment(*models.User, *models.Repository,
*models.Issue, *models.Comment)
Expand Down
4 changes: 4 additions & 0 deletions modules/notification/base/null.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ func (*NullNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *models
func (*NullNotifier) NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest) {
}

// NotifyPullRequestChangeTargetBranch places a place holder function
func (*NullNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string) {
}

// NotifyUpdateComment places a place holder function
func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) {
}
Expand Down
7 changes: 7 additions & 0 deletions modules/notification/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ func NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comm
}
}

// NotifyPullRequestChangeTargetBranch notifies when a pull request's target branch was changed
func NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string) {
for _, notifier := range notifiers {
notifier.NotifyPullRequestChangeTargetBranch(doer, pr, oldBranch)
}
}

// NotifyUpdateComment notifies update comment to notifiers
func NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) {
for _, notifier := range notifiers {
Expand Down
31 changes: 31 additions & 0 deletions modules/notification/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,37 @@ func (*webhookNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mod
}
}

func (m *webhookNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string) {
issue := pr.Issue
if !issue.IsPull {
saitho marked this conversation as resolved.
Show resolved Hide resolved
return
}
var err error

if err = issue.LoadPullRequest(); err != nil {
log.Error("LoadPullRequest failed: %v", err)
return
}
issue.PullRequest.Issue = issue
mode, _ := models.AccessLevel(issue.Poster, issue.Repo)
err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueEdited,
Index: issue.Index,
Changes: &api.ChangesPayload{
Ref: &api.ChangesFromPayload{
From: oldBranch,
},
},
PullRequest: issue.PullRequest.APIFormat(),
Repository: issue.Repo.APIFormat(mode),
Sender: doer.APIFormat(),
})

if err != nil {
log.Error("PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
}
}

func (m *webhookNotifier) NotifyPullRequestReview(pr *models.PullRequest, review *models.Review, comment *models.Comment) {
var reviewHookType models.HookEventType

Expand Down
3 changes: 2 additions & 1 deletion modules/structs/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,11 @@ type ChangesFromPayload struct {
From string `json:"from"`
}

// ChangesPayload FIXME
// ChangesPayload represents the payload information of issue change
type ChangesPayload struct {
Title *ChangesFromPayload `json:"title,omitempty"`
Body *ChangesFromPayload `json:"body,omitempty"`
Ref *ChangesFromPayload `json:"ref,omitempty"`
}

// __________ .__ .__ __________ __
Expand Down
4 changes: 3 additions & 1 deletion options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1033,15 +1033,17 @@ pulls.no_results = No results found.
pulls.nothing_to_compare = These branches are equal. There is no need to create a pull request.
pulls.has_pull_request = `A pull request between these branches already exists: <a href="%[1]s/pulls/%[3]d">%[2]s#%[3]d</a>`
pulls.create = Create Pull Request
pulls.title_desc = wants to merge %[1]d commits from <code>%[2]s</code> into <code>%[3]s</code>
pulls.title_desc = wants to merge %[1]d commits from <code>%[2]s</code> into <code id="branch_target">%[3]s</code>
pulls.merged_title_desc = merged %[1]d commits from <code>%[2]s</code> into <code>%[3]s</code> %[4]s
pulls.change_target_branch_at = `changed target branch from <b>%s</b> to <b>%s</b> %s`
pulls.tab_conversation = Conversation
pulls.tab_commits = Commits
pulls.tab_files = Files Changed
pulls.reopen_to_merge = Please reopen this pull request to perform a merge.
pulls.cant_reopen_deleted_branch = This pull request cannot be reopened because the branch was deleted.
pulls.merged = Merged
pulls.merged_as = The pull request has been merged as <a rel="nofollow" class="ui sha" href="%[1]s"><code>%[2]s</code></a>.
pulls.is_closed = The pull request has been closed.
pulls.has_merged = The pull request has been merged.
pulls.title_wip_desc = `<a href="#">Start the title with <strong>%s</strong></a> to prevent the pull request from being merged accidentally.`
pulls.cannot_merge_work_in_progress = This pull request is marked as a work in progress. Remove the <strong>%s</strong> prefix from the title when it's ready
Expand Down
14 changes: 14 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,19 @@ func commentTag(repo *models.Repository, poster *models.User, issue *models.Issu
return models.CommentTagNone, nil
}

func getBranchData(ctx *context.Context, issue *models.Issue) {
ctx.Data["BaseBranch"] = nil
ctx.Data["HeadBranch"] = nil
ctx.Data["HeadUserName"] = nil
ctx.Data["BaseName"] = ctx.Repo.Repository.OwnerName
if issue.IsPull {
pull := issue.PullRequest
ctx.Data["BaseBranch"] = pull.BaseBranch
ctx.Data["HeadBranch"] = pull.HeadBranch
ctx.Data["HeadUserName"] = pull.MustHeadUserName()
}
}

// ViewIssue render issue view page
func ViewIssue(ctx *context.Context) {
if ctx.Params(":type") == "issues" {
Expand Down Expand Up @@ -885,6 +898,7 @@ func ViewIssue(ctx *context.Context) {
}
}

getBranchData(ctx, issue)
if issue.IsPull {
pull := issue.PullRequest
pull.Issue = issue
Expand Down
Loading