diff options
author | Mario Lubenka <mario.lubenka@googlemail.com> | 2019-12-16 07:20:25 +0100 |
---|---|---|
committer | Lunny Xiao <xiaolunwen@gmail.com> | 2019-12-16 14:20:25 +0800 |
commit | 61db8349041cceceb4ad3233e69613705bd0a128 (patch) | |
tree | 1a7b59c74f2ae7406548181e1d7e611b89a397db | |
parent | 59d6401486627b8f47a0c6b62599e65a40f84c92 (diff) | |
download | gitea-61db8349041cceceb4ad3233e69613705bd0a128.tar.gz gitea-61db8349041cceceb4ad3233e69613705bd0a128.zip |
Change target branch for pull request (#6488)
* Adds functionality to change target branch of created pull requests
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Use const instead of var in JavaScript additions
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Check if branches are equal and if PR already exists before changing target branch
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Make sure to check all commits
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Print error messages for user as error flash message
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Disallow changing target branch of closed or merged pull requests
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Resolve conflicts after merge of upstream/master
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Change order of branch select fields
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Removes duplicate check
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Use ctx.Tr for translations
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Recompile JS
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Use correct translation namespace
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Remove redundant if condition
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Moves most change branch logic into pull service
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Completes comment
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Add Ref to ChangesPayload for logging changed target branches
instead of creating a new struct
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Revert changes to go.mod
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Directly use createComment method
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Return 404 if pull request is not found. Move written check up
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Remove variable declaration
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Return client errors on change pull request target errors
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Return error in commit.HasPreviousCommit
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Adds blank line
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Test patch before persisting new target branch
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Update patch before testing (not working)
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Removes patch calls when changeing pull request target
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Removes unneeded check for base name
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Moves ChangeTargetBranch completely to pull service. Update patch status.
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Set webhook mode after errors were validated
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Update PR in one transaction
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Move logic for check if head is equal with branch to pull model
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Adds missing comment and simplify return
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
* Adjust CreateComment method call
Signed-off-by: Mario Lubenka <mario.lubenka@googlemail.com>
-rw-r--r-- | models/error.go | 55 | ||||
-rw-r--r-- | models/issue_comment.go | 8 | ||||
-rw-r--r-- | models/migrations/migrations.go | 2 | ||||
-rw-r--r-- | models/migrations/v113.go | 23 | ||||
-rw-r--r-- | models/pull.go | 29 | ||||
-rw-r--r-- | modules/git/commit.go | 21 | ||||
-rw-r--r-- | modules/notification/base/notifier.go | 1 | ||||
-rw-r--r-- | modules/notification/base/null.go | 4 | ||||
-rw-r--r-- | modules/notification/notification.go | 7 | ||||
-rw-r--r-- | modules/notification/webhook/webhook.go | 31 | ||||
-rw-r--r-- | modules/structs/hook.go | 3 | ||||
-rw-r--r-- | options/locale/locale_en-US.ini | 4 | ||||
-rw-r--r-- | routers/repo/issue.go | 14 | ||||
-rw-r--r-- | routers/repo/pull.go | 74 | ||||
-rw-r--r-- | routers/routes/routes.go | 3 | ||||
-rw-r--r-- | services/pull/pull.go | 88 | ||||
-rw-r--r-- | templates/repo/issue/view_content/comments.tmpl | 12 | ||||
-rw-r--r-- | templates/repo/issue/view_title.tmpl | 40 | ||||
-rw-r--r-- | web_src/js/index.js | 61 |
19 files changed, 461 insertions, 19 deletions
diff --git a/models/error.go b/models/error.go index a17ff5f9d0..396d7594c8 100644 --- a/models/error.go +++ b/models/error.go @@ -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 @@ -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 @@ -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) +} + // _________ __ // \_ ___ \ ____ _____ _____ ____ _____/ |_ // / \ \/ / _ \ / \ / \_/ __ \ / \ __\ diff --git a/models/issue_comment.go b/models/issue_comment.go index 9f87c049df..aeaee68003 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -82,6 +82,8 @@ const ( CommentTypeLock // Unlocks a previously locked issue CommentTypeUnlock + // Change pull request's target branch + CommentTypeChangeTargetBranch ) // CommentTag defines comment tag type @@ -116,6 +118,8 @@ type Comment struct { Assignee *User `xorm:"-"` OldTitle string NewTitle string + OldRef string + NewRef string DependentIssueID int64 DependentIssue *Issue `xorm:"-"` @@ -517,6 +521,8 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err Content: opts.Content, OldTitle: opts.OldTitle, NewTitle: opts.NewTitle, + OldRef: opts.OldRef, + NewRef: opts.NewRef, DependentIssueID: opts.DependentIssueID, TreePath: opts.TreePath, ReviewID: opts.ReviewID, @@ -673,6 +679,8 @@ type CreateCommentOptions struct { RemovedAssignee bool OldTitle string NewTitle string + OldRef string + NewRef string CommitID int64 CommitSHA string Patch string diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 78fbc18ca5..cbea5a95dd 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -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 diff --git a/models/migrations/v113.go b/models/migrations/v113.go new file mode 100644 index 0000000000..199c02ac98 --- /dev/null +++ b/models/migrations/v113.go @@ -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 +} diff --git a/models/pull.go b/models/pull.go index d703c9a2ee..ba9c575775 100644 --- a/models/pull.go +++ b/models/pull.go @@ -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) +} diff --git a/modules/git/commit.go b/modules/git/commit.go index 0388d5e9be..dfb7adcd1a 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -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 { + 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) diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index 934ee80aa7..48846b3446 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -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) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index a04d0e8caa..bea4e55277 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -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) { } diff --git a/modules/notification/notification.go b/modules/notification/notification.go index ab671fa291..f567552df5 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -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 { diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index ee91a29f02..e0801445d8 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -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 { + 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 diff --git a/modules/structs/hook.go b/modules/structs/hook.go index e036442904..d62c900f42 100644 --- a/modules/structs/hook.go +++ b/modules/structs/hook.go @@ -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"` } // __________ .__ .__ __________ __ diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index c5cfb1663f..97596451bc 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1033,8 +1033,9 @@ 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 @@ -1042,6 +1043,7 @@ 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 diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 7d11ed3537..07292c1993 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -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" { @@ -885,6 +898,7 @@ func ViewIssue(ctx *context.Context) { } } + getBranchData(ctx, issue) if issue.IsPull { pull := issue.PullRequest pull.Issue = issue diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 559d9b267a..f8612d2b3b 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -11,6 +11,7 @@ import ( "crypto/subtle" "fmt" "html" + "net/http" "path" "strings" @@ -467,6 +468,7 @@ func ViewPullCommits(ctx *context.Context) { ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = commits.Len() + getBranchData(ctx, issue) ctx.HTML(200, tplPullCommits) } @@ -596,6 +598,7 @@ func ViewPullFiles(ctx *context.Context) { ctx.ServerError("GetCurrentReview", err) return } + getBranchData(ctx, issue) ctx.HTML(200, tplPullFiles) } @@ -1010,3 +1013,74 @@ func DownloadPullDiffOrPatch(ctx *context.Context, patch bool) { return } } + +// UpdatePullRequestTarget change pull request's target branch +func UpdatePullRequestTarget(ctx *context.Context) { + issue := GetActionIssue(ctx) + pr := issue.PullRequest + if ctx.Written() { + return + } + if !issue.IsPull { + ctx.Error(http.StatusNotFound) + return + } + + if !ctx.IsSigned || (!issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)) { + ctx.Error(http.StatusForbidden) + return + } + + targetBranch := ctx.QueryTrim("target_branch") + if len(targetBranch) == 0 { + ctx.Error(http.StatusNoContent) + return + } + + if err := pull_service.ChangeTargetBranch(pr, ctx.User, targetBranch); err != nil { + if models.IsErrPullRequestAlreadyExists(err) { + err := err.(models.ErrPullRequestAlreadyExists) + + RepoRelPath := ctx.Repo.Owner.Name + "/" + ctx.Repo.Repository.Name + errorMessage := ctx.Tr("repo.pulls.has_pull_request", ctx.Repo.RepoLink, RepoRelPath, err.IssueID) + + ctx.Flash.Error(errorMessage) + ctx.JSON(http.StatusConflict, map[string]interface{}{ + "error": err.Error(), + "user_error": errorMessage, + }) + } else if models.IsErrIssueIsClosed(err) { + errorMessage := ctx.Tr("repo.pulls.is_closed") + + ctx.Flash.Error(errorMessage) + ctx.JSON(http.StatusConflict, map[string]interface{}{ + "error": err.Error(), + "user_error": errorMessage, + }) + } else if models.IsErrPullRequestHasMerged(err) { + errorMessage := ctx.Tr("repo.pulls.has_merged") + + ctx.Flash.Error(errorMessage) + ctx.JSON(http.StatusConflict, map[string]interface{}{ + "error": err.Error(), + "user_error": errorMessage, + }) + } else if models.IsErrBranchesEqual(err) { + errorMessage := ctx.Tr("repo.pulls.nothing_to_compare") + + ctx.Flash.Error(errorMessage) + ctx.JSON(http.StatusBadRequest, map[string]interface{}{ + "error": err.Error(), + "user_error": errorMessage, + }) + } else { + ctx.ServerError("UpdatePullRequestTarget", err) + } + return + } + notification.NotifyPullRequestChangeTargetBranch(ctx.User, pr, targetBranch) + + ctx.JSON(http.StatusOK, map[string]interface{}{ + "base_branch": pr.BaseBranch, + }) +} diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 60fd93df9c..cb4fadbcdb 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -770,6 +770,9 @@ func RegisterRoutes(m *macaron.Macaron) { m.Combo("/compare/*", repo.MustBeNotEmpty, reqRepoCodeReader, repo.SetEditorconfigIfExists). Get(repo.SetDiffViewStyle, repo.CompareDiff). Post(context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, bindIgnErr(auth.CreateIssueForm{}), repo.CompareAndPullRequestPost) + m.Group("/pull", func() { + m.Post("/:index/target_branch", repo.UpdatePullRequestTarget) + }, context.RepoMustNotBeArchived()) m.Group("", func() { m.Group("", func() { diff --git a/services/pull/pull.go b/services/pull/pull.go index e7f4e4eede..fb47df1c3a 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -46,6 +46,94 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 return nil } +// ChangeTargetBranch changes the target branch of this pull request, as the given user. +func ChangeTargetBranch(pr *models.PullRequest, doer *models.User, targetBranch string) (err error) { + // Current target branch is already the same + if pr.BaseBranch == targetBranch { + return nil + } + + if pr.Issue.IsClosed { + return models.ErrIssueIsClosed{ + ID: pr.Issue.ID, + RepoID: pr.Issue.RepoID, + Index: pr.Issue.Index, + } + } + + if pr.HasMerged { + return models.ErrPullRequestHasMerged{ + ID: pr.ID, + IssueID: pr.Index, + HeadRepoID: pr.HeadRepoID, + BaseRepoID: pr.BaseRepoID, + HeadBranch: pr.HeadBranch, + BaseBranch: pr.BaseBranch, + } + } + + // Check if branches are equal + branchesEqual, err := pr.IsHeadEqualWithBranch(targetBranch) + if err != nil { + return err + } + if branchesEqual { + return models.ErrBranchesEqual{ + HeadBranchName: pr.HeadBranch, + BaseBranchName: targetBranch, + } + } + + // Check if pull request for the new target branch already exists + existingPr, err := models.GetUnmergedPullRequest(pr.HeadRepoID, pr.BaseRepoID, pr.HeadBranch, targetBranch) + if existingPr != nil { + return models.ErrPullRequestAlreadyExists{ + ID: existingPr.ID, + IssueID: existingPr.Index, + HeadRepoID: existingPr.HeadRepoID, + BaseRepoID: existingPr.BaseRepoID, + HeadBranch: existingPr.HeadBranch, + BaseBranch: existingPr.BaseBranch, + } + } + if err != nil && !models.IsErrPullRequestNotExist(err) { + return err + } + + // Set new target branch + oldBranch := pr.BaseBranch + pr.BaseBranch = targetBranch + + // Refresh patch + if err := TestPatch(pr); err != nil { + return err + } + + // Update target branch, PR diff and status + // This is the same as checkAndUpdateStatus in check service, but also updates base_branch + if pr.Status == models.PullRequestStatusChecking { + pr.Status = models.PullRequestStatusMergeable + } + if err := pr.UpdateCols("status, conflicted_files, base_branch"); err != nil { + return err + } + + // Create comment + options := &models.CreateCommentOptions{ + Type: models.CommentTypeChangeTargetBranch, + Doer: doer, + Repo: pr.Issue.Repo, + Issue: pr.Issue, + OldRef: oldBranch, + NewRef: targetBranch, + } + if _, err = models.CreateComment(options); err != nil { + return fmt.Errorf("CreateChangeTargetBranchComment: %v", err) + } + + return nil +} + func checkForInvalidation(requests models.PullRequestList, repoID int64, doer *models.User, branch string) error { repo, err := models.GetRepositoryByID(repoID) if err != nil { diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 49d2e9bc5f..d8a90caa85 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -6,7 +6,7 @@ 5 = COMMENT_REF, 6 = PULL_REF, 7 = COMMENT_LABEL, 12 = START_TRACKING, 13 = STOP_TRACKING, 14 = ADD_TIME_MANUAL, 16 = ADDED_DEADLINE, 17 = MODIFIED_DEADLINE, 18 = REMOVED_DEADLINE, 19 = ADD_DEPENDENCY, 20 = REMOVE_DEPENDENCY, 21 = CODE, - 22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED --> + 22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED, 25 = TARGET_BRANCH_CHANGED --> {{if eq .Type 0}} <div class="comment" id="{{.HashTag}}"> {{if .OriginalAuthor }} @@ -411,5 +411,15 @@ {{$.i18n.Tr "repo.issues.unlock_comment" $createdStr | Safe}} </span> </div> + {{else if eq .Type 25}} + <div class="event"> + <span class="octicon octicon-primitive-dot"></span> + <a class="ui avatar image" href="{{.Poster.HomeLink}}"> + <img src="{{.Poster.RelAvatarLink}}"> + </a> + <span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a> + {{$.i18n.Tr "repo.pulls.change_target_branch_at" (.OldRef|Escape) (.NewRef|Escape) $createdStr | Safe}} + </span> + </div> {{end}} {{end}} diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl index e97ca3265c..635638f325 100644 --- a/templates/repo/issue/view_title.tmpl +++ b/templates/repo/issue/view_title.tmpl @@ -11,7 +11,7 @@ <div class="edit-zone text right"> <div id="edit-title" class="ui basic green not-in-edit button">{{.i18n.Tr "repo.issues.edit"}}</div> <div id="cancel-edit-title" class="ui basic blue in-edit button" style="display: none">{{.i18n.Tr "repo.issues.cancel"}}</div> - <div id="save-edit-title" class="ui green in-edit button" style="display: none" data-update-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/title">{{.i18n.Tr "repo.issues.save"}}</div> + <div id="save-edit-title" class="ui green in-edit button" style="display: none" data-update-url="{{$.RepoLink}}/issues/{{.Issue.Index}}/title" {{if .Issue.IsPull}}data-target-update-url="{{$.RepoLink}}/pull/{{.Issue.Index}}/target_branch"{{end}}>{{.i18n.Tr "repo.issues.save"}}</div> </div> </div> {{end}} @@ -36,12 +36,42 @@ {{end}} {{else}} {{if .Issue.OriginalAuthor }} - {{.Issue.OriginalAuthor}} - <span class="pull-desc">{{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}}</span> + <span id="pull-desc" class="pull-desc">{{.Issue.OriginalAuthor}} {{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}}</span> {{else}} - <a {{if gt .Issue.Poster.ID 0}}href="{{.Issue.Poster.HomeLink}}"{{end}}>{{.Issue.Poster.GetDisplayName}}</a> - <span class="pull-desc">{{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}}</span> + <span id="pull-desc" class="pull-desc"> + <a {{if gt .Issue.Poster.ID 0}}href="{{.Issue.Poster.HomeLink}}"{{end}}>{{.Issue.Poster.GetDisplayName}}</a> + {{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}} + </span> {{end}} + <span id="pull-desc-edit" style="display: none"> + <div class="ui floating filter dropdown"> + <div class="ui basic small button"> + <span class="text">{{.i18n.Tr "repo.pulls.compare_compare"}}: {{$.HeadTarget}}</span> + </div> + </div> + <i class="octicon octicon-arrow-right"></i> + <div class="ui floating filter dropdown" data-no-results="{{.i18n.Tr "repo.pulls.no_results"}}"> + <div class="ui basic small button"> + <span class="text" id="pull-target-branch" data-basename="{{$.BaseName}}" data-branch="{{$.BaseBranch}}">{{.i18n.Tr "repo.pulls.compare_base"}}: {{$.BaseName}}:{{$.BaseBranch}}</span> + <i class="dropdown icon"></i> + </div> + <div class="menu"> + <div class="ui icon search input"> + <i class="filter icon"></i> + <input name="search" placeholder="{{.i18n.Tr "repo.pulls.filter_branch"}}..."> + </div> + <div class="scrolling menu" id="branch-select"> + {{range .Branches}} + {{ $sameBase := ne $.BaseName $.HeadUserName }} + {{ $differentBranch := ne . $.HeadBranch }} + {{ if or $sameBase $differentBranch }} + <div class="item {{if eq $.BaseBranch .}}selected{{end}}" data-branch="{{.}}">{{$.BaseName}}{{if $.HeadRepo}}/{{$.HeadRepo}}{{end}}:{{.}}</div> + {{ end }} + {{end}} + </div> + </div> + </div> + </span> {{end}} {{else}} {{ $createdStr:= TimeSinceUnix .Issue.CreatedUnix $.Lang }} diff --git a/web_src/js/index.js b/web_src/js/index.js index a7aa0d52c4..a310243c08 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -731,27 +731,66 @@ function initRepository() { $issueTitle.toggle(); $('.not-in-edit').toggle(); $('#edit-title-input').toggle(); + $('#pull-desc').toggle(); + $('#pull-desc-edit').toggle(); $('.in-edit').toggle(); $editInput.focus(); return false; }; + + const changeBranchSelect = function () { + const selectionTextField = $('#pull-target-branch'); + + const baseName = selectionTextField.data('basename'); + const branchNameNew = $(this).data('branch'); + const branchNameOld = selectionTextField.data('branch'); + + // Replace branch name to keep translation from HTML template + selectionTextField.html(selectionTextField.html().replace( + `${baseName}:${branchNameOld}`, + `${baseName}:${branchNameNew}` + )); + selectionTextField.data('branch', branchNameNew); // update branch name in setting + }; + $('#branch-select > .item').click(changeBranchSelect); + $('#edit-title').click(editTitleToggle); $('#cancel-edit-title').click(editTitleToggle); $('#save-edit-title').click(editTitleToggle).click(function () { + const pullrequest_targetbranch_change = function (update_url) { + const targetBranch = $('#pull-target-branch').data('branch'); + const $branchTarget = $('#branch_target'); + if (targetBranch === $branchTarget.text()) { + return false; + } + $.post(update_url, { + _csrf: csrf, + target_branch: targetBranch + }) + .success((data) => { + $branchTarget.text(data.base_branch); + }) + .always(() => { + reload(); + }); + }; + + const pullrequest_target_update_url = $(this).data('target-update-url'); if ($editInput.val().length === 0 || $editInput.val() === $issueTitle.text()) { $editInput.val($issueTitle.text()); - return false; + pullrequest_targetbranch_change(pullrequest_target_update_url); + } else { + $.post($(this).data('update-url'), { + _csrf: csrf, + title: $editInput.val() + }, + (data) => { + $editInput.val(data.title); + $issueTitle.text(data.title); + pullrequest_targetbranch_change(pullrequest_target_update_url); + reload(); + }); } - - $.post($(this).data('update-url'), { - _csrf: csrf, - title: $editInput.val() - }, - (data) => { - $editInput.val(data.title); - $issueTitle.text(data.title); - reload(); - }); return false; }); |