From 61db8349041cceceb4ad3233e69613705bd0a128 Mon Sep 17 00:00:00 2001 From: Mario Lubenka Date: Mon, 16 Dec 2019 07:20:25 +0100 Subject: [PATCH] Change target branch for pull request (#6488) * Adds functionality to change target branch of created pull requests Signed-off-by: Mario Lubenka * Use const instead of var in JavaScript additions Signed-off-by: Mario Lubenka * Check if branches are equal and if PR already exists before changing target branch Signed-off-by: Mario Lubenka * Make sure to check all commits Signed-off-by: Mario Lubenka * Print error messages for user as error flash message Signed-off-by: Mario Lubenka * Disallow changing target branch of closed or merged pull requests Signed-off-by: Mario Lubenka * Resolve conflicts after merge of upstream/master Signed-off-by: Mario Lubenka * Change order of branch select fields Signed-off-by: Mario Lubenka * Removes duplicate check Signed-off-by: Mario Lubenka * Use ctx.Tr for translations Signed-off-by: Mario Lubenka * Recompile JS Signed-off-by: Mario Lubenka * Use correct translation namespace Signed-off-by: Mario Lubenka * Remove redundant if condition Signed-off-by: Mario Lubenka * Moves most change branch logic into pull service Signed-off-by: Mario Lubenka * Completes comment Signed-off-by: Mario Lubenka * Add Ref to ChangesPayload for logging changed target branches instead of creating a new struct Signed-off-by: Mario Lubenka * Revert changes to go.mod Signed-off-by: Mario Lubenka * Directly use createComment method Signed-off-by: Mario Lubenka * Return 404 if pull request is not found. Move written check up Signed-off-by: Mario Lubenka * Remove variable declaration Signed-off-by: Mario Lubenka * Return client errors on change pull request target errors Signed-off-by: Mario Lubenka * Return error in commit.HasPreviousCommit Signed-off-by: Mario Lubenka * Adds blank line Signed-off-by: Mario Lubenka * Test patch before persisting new target branch Signed-off-by: Mario Lubenka * Update patch before testing (not working) Signed-off-by: Mario Lubenka * Removes patch calls when changeing pull request target Signed-off-by: Mario Lubenka * Removes unneeded check for base name Signed-off-by: Mario Lubenka * Moves ChangeTargetBranch completely to pull service. Update patch status. Signed-off-by: Mario Lubenka * Set webhook mode after errors were validated Signed-off-by: Mario Lubenka * Update PR in one transaction Signed-off-by: Mario Lubenka * Move logic for check if head is equal with branch to pull model Signed-off-by: Mario Lubenka * Adds missing comment and simplify return Signed-off-by: Mario Lubenka * Adjust CreateComment method call Signed-off-by: Mario Lubenka --- models/error.go | 55 ++++++++++++ models/issue_comment.go | 8 ++ models/migrations/migrations.go | 2 + models/migrations/v113.go | 23 +++++ models/pull.go | 29 ++++++ modules/git/commit.go | 21 +++++ modules/notification/base/notifier.go | 1 + modules/notification/base/null.go | 4 + modules/notification/notification.go | 7 ++ modules/notification/webhook/webhook.go | 31 +++++++ modules/structs/hook.go | 3 +- options/locale/locale_en-US.ini | 4 +- routers/repo/issue.go | 14 +++ routers/repo/pull.go | 74 ++++++++++++++++ routers/routes/routes.go | 3 + services/pull/pull.go | 88 +++++++++++++++++++ .../repo/issue/view_content/comments.tmpl | 12 ++- templates/repo/issue/view_title.tmpl | 40 +++++++-- web_src/js/index.js | 61 ++++++++++--- 19 files changed, 461 insertions(+), 19 deletions(-) create mode 100644 models/migrations/v113.go diff --git a/models/error.go b/models/error.go index a17ff5f9d..396d7594c 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 9f87c049d..aeaee6800 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 78fbc18ca..cbea5a95d 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 000000000..199c02ac9 --- /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 d703c9a2e..ba9c57577 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 0388d5e9b..dfb7adcd1 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 934ee80aa..48846b344 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 a04d0e8ca..bea4e5527 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 ab671fa29..f567552df 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 ee91a29f0..e0801445d 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 e03644290..d62c900f4 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 c5cfb1663..97596451b 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: %[2]s#%[3]d` pulls.create = Create Pull Request -pulls.title_desc = wants to merge %[1]d commits from %[2]s into %[3]s +pulls.title_desc = wants to merge %[1]d commits from %[2]s into %[3]s pulls.merged_title_desc = merged %[1]d commits from %[2]s into %[3]s %[4]s +pulls.change_target_branch_at = `changed target branch from %s to %s %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 %[2]s. +pulls.is_closed = The pull request has been closed. pulls.has_merged = The pull request has been merged. pulls.title_wip_desc = `Start the title with %s 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 %s prefix from the title when it's ready diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 7d11ed353..07292c199 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 559d9b267..f8612d2b3 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 60fd93df9..cb4fadbcd 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 e7f4e4eed..fb47df1c3 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 49d2e9bc5..d8a90caa8 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}}
{{if .OriginalAuthor }} @@ -411,5 +411,15 @@ {{$.i18n.Tr "repo.issues.unlock_comment" $createdStr | Safe}}
+ {{else if eq .Type 25}} +
+ + + + + {{.Poster.Name}} + {{$.i18n.Tr "repo.pulls.change_target_branch_at" (.OldRef|Escape) (.NewRef|Escape) $createdStr | Safe}} + +
{{end}} {{end}} diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl index e97ca3265..635638f32 100644 --- a/templates/repo/issue/view_title.tmpl +++ b/templates/repo/issue/view_title.tmpl @@ -11,7 +11,7 @@
{{.i18n.Tr "repo.issues.edit"}}
- +
{{end}} @@ -36,12 +36,42 @@ {{end}} {{else}} {{if .Issue.OriginalAuthor }} - {{.Issue.OriginalAuthor}} - {{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}} + {{.Issue.OriginalAuthor}} {{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}} {{else}} - {{.Issue.Poster.GetDisplayName}} - {{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}} + + {{.Issue.Poster.GetDisplayName}} + {{$.i18n.Tr "repo.pulls.title_desc" .NumCommits .HeadTarget .BaseTarget | Str2html}} + {{end}} + {{end}} {{else}} {{ $createdStr:= TimeSinceUnix .Issue.CreatedUnix $.Lang }} diff --git a/web_src/js/index.js b/web_src/js/index.js index a7aa0d52c..a310243c0 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; });