From 902e5a864c2818bf224e95a4379f7ec883c12c67 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 4 Jan 2022 17:35:24 +0800 Subject: [PATCH 01/28] Allow custom default merge message with .gitea/MERGE_MESSAGE__TEMPLATE.md --- modules/git/commit.go | 18 +++++++ routers/api/v1/repo/pull.go | 16 +++++- routers/web/repo/issue.go | 24 ++++----- routers/web/repo/pull.go | 18 +++++++ services/pull/merge.go | 54 +++++++++++++++++++++ templates/repo/issue/view_content/pull.tmpl | 3 -- 6 files changed, 118 insertions(+), 15 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 8337e54fef27d..6d056a8ed68df 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -306,6 +306,24 @@ func (c *Commit) HasFile(filename string) (bool, error) { return true, nil } +// GetFileContent reads a file content as a string +func (c *Commit) GetFileContent(filename string) (string, bool) { + entry, err := c.GetTreeEntryByPath(filename) + if err != nil { + return "", false + } + r, err := entry.Blob().DataAsync() + if err != nil { + return "", false + } + defer r.Close() + bytes, err := io.ReadAll(r) + if err != nil { + return "", false + } + return string(bytes), true +} + // GetSubModules get all the sub modules of current revision git tree func (c *Commit) GetSubModules() (*ObjectCache, error) { if c.submoduleCache != nil { diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 91bb57f3fd79a..5e0cc50c24f00 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -823,7 +823,21 @@ func MergePullRequest(ctx *context.APIContext) { } } - if err := pull_service.Merge(pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil { + if len(form.Do) == 0 { + form.Do = string(repo_model.MergeStyleMerge) + } + + message := strings.TrimSpace(form.MergeTitleField) + if len(message) == 0 { + message = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pr, repo_model.MergeStyle(form.Do)) + } + + form.MergeMessageField = strings.TrimSpace(form.MergeMessageField) + if len(form.MergeMessageField) > 0 { + message += "\n\n" + form.MergeMessageField + } + + if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Error(http.StatusMethodNotAllowed, "Invalid merge style", fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do))) } else if models.IsErrMergeConflicts(err) { diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 620b76f46d360..99a23a4f3557c 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -712,8 +712,6 @@ func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull } func getFileContentFromDefaultBranch(ctx *context.Context, filename string) (string, bool) { - var bytes []byte - if ctx.Repo.Commit == nil { var err error ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) @@ -734,7 +732,7 @@ func getFileContentFromDefaultBranch(ctx *context.Context, filename string) (str return "", false } defer r.Close() - bytes, err = io.ReadAll(r) + bytes, err := io.ReadAll(r) if err != nil { return "", false } @@ -1574,26 +1572,30 @@ func ViewIssue(ctx *context.Context) { } prConfig := prUnit.PullRequestsConfig() + var mergeStyle repo_model.MergeStyle // Check correct values and select default if ms, ok := ctx.Data["MergeStyle"].(repo_model.MergeStyle); !ok || !prConfig.IsMergeStyleAllowed(ms) { defaultMergeStyle := prConfig.GetDefaultMergeStyle() if prConfig.IsMergeStyleAllowed(defaultMergeStyle) && !ok { - ctx.Data["MergeStyle"] = defaultMergeStyle + mergeStyle = defaultMergeStyle } else if prConfig.AllowMerge { - ctx.Data["MergeStyle"] = repo_model.MergeStyleMerge + mergeStyle = repo_model.MergeStyleMerge } else if prConfig.AllowRebase { - ctx.Data["MergeStyle"] = repo_model.MergeStyleRebase + mergeStyle = repo_model.MergeStyleRebase } else if prConfig.AllowRebaseMerge { - ctx.Data["MergeStyle"] = repo_model.MergeStyleRebaseMerge + mergeStyle = repo_model.MergeStyleRebaseMerge } else if prConfig.AllowSquash { - ctx.Data["MergeStyle"] = repo_model.MergeStyleSquash + mergeStyle = repo_model.MergeStyleSquash } else if prConfig.AllowManualMerge { - ctx.Data["MergeStyle"] = repo_model.MergeStyleManuallyMerged - } else { - ctx.Data["MergeStyle"] = "" + mergeStyle = repo_model.MergeStyleManuallyMerged } } + + ctx.Data["MergeStyle"] = mergeStyle + + ctx.Data["DefaultMergeMessage"] = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pull, mergeStyle) + if err = pull.LoadProtectedBranch(); err != nil { ctx.ServerError("LoadProtectedBranch", err) return diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a0b356773857a..089dfb4ea8053 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -956,6 +956,24 @@ func MergePullRequest(ctx *context.Context) { return } + message := strings.TrimSpace(form.MergeTitleField) + if len(message) == 0 { + message = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pr, repo_model.MergeStyle(form.Do)) + } + + form.MergeMessageField = strings.TrimSpace(form.MergeMessageField) + if len(form.MergeMessageField) > 0 { + message += "\n\n" + form.MergeMessageField + } + + pr.Issue = issue + pr.Issue.Repo = ctx.Repo.Repository + + noDeps, err := models.IssueNoDependenciesLeft(ctx, issue) + if err != nil { + return + } + if err := pull_service.Merge(pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) diff --git a/services/pull/merge.go b/services/pull/merge.go index 8cc4d8888845d..59cd99fa21b93 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -25,14 +25,68 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" ) +// GetDefaultMergeMessage returns default message used when merging pull request +func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, mergeStyle repo_model.MergeStyle) string { + if err := pr.LoadHeadRepo(); err != nil { + log.Error("LoadHeadRepo[%d]: %v", pr.HeadRepoID, err) + return "" + } + if err := pr.LoadIssue(); err != nil { + log.Error("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err) + return "" + } + if err := pr.LoadBaseRepo(); err != nil { + log.Error("LoadBaseRepo: %v", err) + return "" + } + + issueReference := "#" + if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) { + issueReference = "!" + } + + if mergeStyle != "" { + templateFilepath := fmt.Sprintf(".gitea/MERGE_MESSAGE_%s_TEMPLATE.md", mergeStyle) + commit, err := baseGitRepo.GetBranchCommit(pr.BaseRepo.DefaultBranch) + if err != nil { + log.Error("GetBranchCommit: %v", err) + return "" + } + templateContent, ok := commit.GetFileContent(templateFilepath) + if ok { + var meta api.IssueTemplate + templateBody, err := markdown.ExtractMetadata(templateContent, &meta) + if err == nil { + return templateBody + } + log.Error("could not extract metadata from %s [%s]: %v", templateFilepath, pr.BaseRepo.FullName(), err) + } + } + + if mergeStyle == repo_model.MergeStyleSquash { + if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) { + return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index) + } + return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index) + } + + if pr.BaseRepoID == pr.HeadRepoID { + return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch) + } + + return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch) +} + // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) error { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 8c43ace32e134..e5519617cabf8 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -377,9 +377,6 @@
-
- -
From 9ec30d35cf40a98425d557212ae316d2a88859a7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 15 Jan 2022 13:06:55 +0800 Subject: [PATCH 02/28] Some improvements --- .../content/doc/usage/issue-pull-request-templates.en-us.md | 6 ++++++ modules/git/commit.go | 2 +- services/pull/merge.go | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/content/doc/usage/issue-pull-request-templates.en-us.md b/docs/content/doc/usage/issue-pull-request-templates.en-us.md index 218b8a36427b7..06100de0f38d1 100644 --- a/docs/content/doc/usage/issue-pull-request-templates.en-us.md +++ b/docs/content/doc/usage/issue-pull-request-templates.en-us.md @@ -42,6 +42,12 @@ Possible file names for PR templates: - `.gitea/pull_request_template.md` - `.github/PULL_REQUEST_TEMPLATE.md` - `.github/pull_request_template.md` +- `.gitea/MERGE_MESSAGE_TEMPLATE.md` +- `.gitea/REBASE_MESSAGE_TEMPLATE.md` +- `.gitea/REBASE-MERGE_MESSAGE_TEMPLATE.md` +- `.gitea/SQUASH_MESSAGE_TEMPLATE.md` +- `.gitea/MANUALLY-MERGED_MESSAGE_TEMPLATE.md` +- `.gitea/REBASE-UPDATE-ONLY_MESSAGE_TEMPLATE.md` Additionally, the New Issue page URL can be suffixed with `?title=Issue+Title&body=Issue+Text` and the form will be populated with those strings. Those strings will be used instead of the template if there is one. diff --git a/modules/git/commit.go b/modules/git/commit.go index 6d056a8ed68df..6e73404dc237d 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -306,7 +306,7 @@ func (c *Commit) HasFile(filename string) (bool, error) { return true, nil } -// GetFileContent reads a file content as a string +// GetFileContent reads a file content as a string or returns false if this was not possible func (c *Commit) GetFileContent(filename string) (string, bool) { entry, err := c.GetTreeEntryByPath(filename) if err != nil { diff --git a/services/pull/merge.go b/services/pull/merge.go index 59cd99fa21b93..f35cc0288f466 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -56,7 +56,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, } if mergeStyle != "" { - templateFilepath := fmt.Sprintf(".gitea/MERGE_MESSAGE_%s_TEMPLATE.md", mergeStyle) + templateFilepath := fmt.Sprintf(".gitea/%s_MESSAGE_TEMPLATE.md", strings.ToUpper(string(mergeStyle))) commit, err := baseGitRepo.GetBranchCommit(pr.BaseRepo.DefaultBranch) if err != nil { log.Error("GetBranchCommit: %v", err) From c65ad5c8b6cd09d504b7870c764c67d23072226f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 9 Feb 2022 16:25:26 +0800 Subject: [PATCH 03/28] Follow some advices --- .../doc/usage/issue-pull-request-templates.en-us.md | 12 ++++++------ services/pull/merge.go | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/content/doc/usage/issue-pull-request-templates.en-us.md b/docs/content/doc/usage/issue-pull-request-templates.en-us.md index 06100de0f38d1..22ceed565f7aa 100644 --- a/docs/content/doc/usage/issue-pull-request-templates.en-us.md +++ b/docs/content/doc/usage/issue-pull-request-templates.en-us.md @@ -42,12 +42,12 @@ Possible file names for PR templates: - `.gitea/pull_request_template.md` - `.github/PULL_REQUEST_TEMPLATE.md` - `.github/pull_request_template.md` -- `.gitea/MERGE_MESSAGE_TEMPLATE.md` -- `.gitea/REBASE_MESSAGE_TEMPLATE.md` -- `.gitea/REBASE-MERGE_MESSAGE_TEMPLATE.md` -- `.gitea/SQUASH_MESSAGE_TEMPLATE.md` -- `.gitea/MANUALLY-MERGED_MESSAGE_TEMPLATE.md` -- `.gitea/REBASE-UPDATE-ONLY_MESSAGE_TEMPLATE.md` +- `.gitea/default_merge_message/MERGE_TEMPLATE.md` +- `.gitea/default_merge_message/REBASE_TEMPLATE.md` +- `.gitea/default_merge_message/REBASE-MERGE_TEMPLATE.md` +- `.gitea/default_merge_message/SQUASH_TEMPLATE.md` +- `.gitea/default_merge_message/MANUALLY-MERGED_TEMPLATE.md` +- `.gitea/default_merge_message/REBASE-UPDATE-ONLY_TEMPLATE.md` Additionally, the New Issue page URL can be suffixed with `?title=Issue+Title&body=Issue+Text` and the form will be populated with those strings. Those strings will be used instead of the template if there is one. diff --git a/services/pull/merge.go b/services/pull/merge.go index f35cc0288f466..2c698175b6649 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -56,7 +56,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, } if mergeStyle != "" { - templateFilepath := fmt.Sprintf(".gitea/%s_MESSAGE_TEMPLATE.md", strings.ToUpper(string(mergeStyle))) + templateFilepath := fmt.Sprintf(".gitea/default_merge_message/%s_TEMPLATE.md", strings.ToUpper(string(mergeStyle))) commit, err := baseGitRepo.GetBranchCommit(pr.BaseRepo.DefaultBranch) if err != nil { log.Error("GetBranchCommit: %v", err) @@ -73,6 +73,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, } } + // Squash merge has a different from other styles. if mergeStyle == repo_model.MergeStyleSquash { if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) { return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index) From 16f0514771de24f75f85c9822d166fc3b28bab0b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 10 Feb 2022 17:23:30 +0800 Subject: [PATCH 04/28] Fix bug --- routers/web/repo/issue.go | 1 + services/pull/merge.go | 30 ++++++++++++++++++------------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 99a23a4f3557c..5c6caaa5acb71 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1595,6 +1595,7 @@ func ViewIssue(ctx *context.Context) { ctx.Data["MergeStyle"] = mergeStyle ctx.Data["DefaultMergeMessage"] = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pull, mergeStyle) + ctx.Data["DefaultSquashMergeMessage"] = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pull, repo_model.MergeStyleSquash) if err = pull.LoadProtectedBranch(); err != nil { ctx.ServerError("LoadProtectedBranch", err) diff --git a/services/pull/merge.go b/services/pull/merge.go index 2c698175b6649..6cbece463cc90 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" "time" @@ -25,14 +26,14 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" - api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" + + "github.com/unknwon/com" ) // GetDefaultMergeMessage returns default message used when merging pull request @@ -64,21 +65,26 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, } templateContent, ok := commit.GetFileContent(templateFilepath) if ok { - var meta api.IssueTemplate - templateBody, err := markdown.ExtractMetadata(templateContent, &meta) - if err == nil { - return templateBody - } - log.Error("could not extract metadata from %s [%s]: %v", templateFilepath, pr.BaseRepo.FullName(), err) + return com.Expand(templateContent, + map[string]string{ + "BaseRepoOwner": pr.BaseRepo.OwnerName, + "BaseRepoName": pr.BaseRepo.Name, + "BaseBranch": pr.BaseBranch, + "HeadRepoOwner": pr.HeadRepo.OwnerName, + "HeadRepoName": pr.HeadRepo.Name, + "HeadBranch": pr.HeadBranch, + "PullRequestTitle": pr.Issue.Title, + "PullRequestBody": pr.Issue.Content, + "PullRequestPoster": pr.Issue.Poster.Name, + "PullRequestIndex": strconv.FormatInt(pr.Index, 10), + "IssueReference": issueReference, + }) } } // Squash merge has a different from other styles. if mergeStyle == repo_model.MergeStyleSquash { - if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) { - return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index) - } - return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index) + return fmt.Sprintf("%s (%s%d)", pr.Issue.Title, issueReference, pr.Issue.Index) } if pr.BaseRepoID == pr.HeadRepoID { From 699989ed4d7e48356b6bfc09af9aac33c052ac85 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 12 Feb 2022 02:36:13 +0800 Subject: [PATCH 05/28] Fix bug --- .../issue-pull-request-templates.en-us.md | 18 ++++++++ services/pull/merge.go | 44 ++++++++++++------- 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/docs/content/doc/usage/issue-pull-request-templates.en-us.md b/docs/content/doc/usage/issue-pull-request-templates.en-us.md index 22ceed565f7aa..47b5334a4f304 100644 --- a/docs/content/doc/usage/issue-pull-request-templates.en-us.md +++ b/docs/content/doc/usage/issue-pull-request-templates.en-us.md @@ -42,6 +42,9 @@ Possible file names for PR templates: - `.gitea/pull_request_template.md` - `.github/PULL_REQUEST_TEMPLATE.md` - `.github/pull_request_template.md` + +Possible file names for PR default merge message templates: + - `.gitea/default_merge_message/MERGE_TEMPLATE.md` - `.gitea/default_merge_message/REBASE_TEMPLATE.md` - `.gitea/default_merge_message/REBASE-MERGE_TEMPLATE.md` @@ -49,6 +52,21 @@ Possible file names for PR templates: - `.gitea/default_merge_message/MANUALLY-MERGED_TEMPLATE.md` - `.gitea/default_merge_message/REBASE-UPDATE-ONLY_TEMPLATE.md` +You can use some variables in these template with `{}` + +- BaseRepoOwner: Base repository owner name of this pull request +- BaseRepoName: Base repository name of this pull request +- BaseBranch: Base repository target branch name of this pull request +- HeadRepoOwner: Head repository owner name of this pull request +- HeadRepoName: Head repository name of this pull request +- HeadBranch: Head repository branch name of this pull request +- PullRequestTitle: Pull request's title +- PullRequestDescription: Pull request's description +- PullRequestPoster: Pull request's poster name +- PullRequestIndex: Pull request's index number +- IssueReferenceChar: return # if it's internal tracker or ! for external tracker +- ClosedIssueIndexes: return a string contains all issues which will be closed by this pull request i.e. `#1, #2` + Additionally, the New Issue page URL can be suffixed with `?title=Issue+Title&body=Issue+Text` and the form will be populated with those strings. Those strings will be used instead of the template if there is one. ## Issue Template Directory diff --git a/services/pull/merge.go b/services/pull/merge.go index 6cbece463cc90..eb5adc731d2bd 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -51,8 +51,9 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, return "" } + isExternalTracker := pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) issueReference := "#" - if pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) { + if isExternalTracker { issueReference = "!" } @@ -65,20 +66,33 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, } templateContent, ok := commit.GetFileContent(templateFilepath) if ok { - return com.Expand(templateContent, - map[string]string{ - "BaseRepoOwner": pr.BaseRepo.OwnerName, - "BaseRepoName": pr.BaseRepo.Name, - "BaseBranch": pr.BaseBranch, - "HeadRepoOwner": pr.HeadRepo.OwnerName, - "HeadRepoName": pr.HeadRepo.Name, - "HeadBranch": pr.HeadBranch, - "PullRequestTitle": pr.Issue.Title, - "PullRequestBody": pr.Issue.Content, - "PullRequestPoster": pr.Issue.Poster.Name, - "PullRequestIndex": strconv.FormatInt(pr.Index, 10), - "IssueReference": issueReference, - }) + vars := map[string]string{ + "BaseRepoOwner": pr.BaseRepo.OwnerName, + "BaseRepoName": pr.BaseRepo.Name, + "BaseBranch": pr.BaseBranch, + "HeadRepoOwner": pr.HeadRepo.OwnerName, + "HeadRepoName": pr.HeadRepo.Name, + "HeadBranch": pr.HeadBranch, + "PullRequestTitle": pr.Issue.Title, + "PullRequestDescription": pr.Issue.Content, + "PullRequestPoster": pr.Issue.Poster.Name, + "PullRequestIndex": strconv.FormatInt(pr.Index, 10), + "IssueReferenceChar": issueReference, + } + refs, err := pr.ResolveCrossReferences() + if err == nil { + closeIssueIndexes := make([]string, 0, len(refs)) + for _, ref := range refs { + if ref.RefAction == references.XRefActionCloses { + closeIssueIndexes = append(closeIssueIndexes, fmt.Sprintf("%s%d", issueReference, ref.Issue.Index)) + } + } + if len(closeIssueIndexes) > 0 { + vars["ClosedIssueIndexes"] = strings.Join(closeIssueIndexes, ", ") + } + } + + return com.Expand(templateContent, vars) } } From d5ecd4609878f7ac3a95095f2367b5eee7f619bb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 16 Feb 2022 14:02:34 +0800 Subject: [PATCH 06/28] Fix lint --- services/pull/pull_test.go | 49 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index 81627ebb77bcf..796c873e6d6db 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -8,6 +8,12 @@ package pull import ( "testing" + "code.gitea.io/gitea/models" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" "github.com/stretchr/testify/assert" ) @@ -29,3 +35,46 @@ func TestPullRequest_CommitMessageTrailersPattern(t *testing.T) { assert.True(t, commitMessageTrailersPattern.MatchString("Additional whitespace is accepted.\n\nSigned-off-by \t : \tBob ")) assert.True(t, commitMessageTrailersPattern.MatchString("Folded value.\n\nFolded-trailer: This is\n a folded\n trailer value\nOther-Trailer: Value")) } + +func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + pr := unittest.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 2}).(*models.PullRequest) + + assert.NoError(t, pr.LoadBaseRepo()) + gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) + assert.NoError(t, err) + defer gitRepo.Close() + + assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) + + pr.BaseRepoID = 1 + pr.HeadRepoID = 2 + assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) +} + +func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + externalTracker := repo_model.RepoUnit{ + Type: unit.TypeExternalTracker, + Config: &repo_model.ExternalTrackerConfig{ + ExternalTrackerFormat: "https://someurl.com/{user}/{repo}/{issue}", + }, + } + baseRepo := &repo_model.Repository{Name: "testRepo", ID: 1} + baseRepo.Owner = &user_model.User{Name: "testOwner"} + baseRepo.Units = []*repo_model.RepoUnit{&externalTracker} + + pr := unittest.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 2, BaseRepo: baseRepo}).(*models.PullRequest) + + assert.NoError(t, pr.LoadBaseRepo()) + gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) + assert.NoError(t, err) + defer gitRepo.Close() + + assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) + + pr.BaseRepoID = 1 + pr.HeadRepoID = 2 + assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) +} From e2611b4c42bdc31cb0d1d3985bba7d20489d72e2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 16 Feb 2022 14:40:11 +0800 Subject: [PATCH 07/28] Fix close comment --- docs/content/doc/usage/issue-pull-request-templates.en-us.md | 2 +- services/pull/merge.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/doc/usage/issue-pull-request-templates.en-us.md b/docs/content/doc/usage/issue-pull-request-templates.en-us.md index 47b5334a4f304..a179c7184bdd7 100644 --- a/docs/content/doc/usage/issue-pull-request-templates.en-us.md +++ b/docs/content/doc/usage/issue-pull-request-templates.en-us.md @@ -65,7 +65,7 @@ You can use some variables in these template with `{}` - PullRequestPoster: Pull request's poster name - PullRequestIndex: Pull request's index number - IssueReferenceChar: return # if it's internal tracker or ! for external tracker -- ClosedIssueIndexes: return a string contains all issues which will be closed by this pull request i.e. `#1, #2` +- ClosedIssueIndexes: return a string contains all issues which will be closed by this pull request i.e. `close #1, close #2` Additionally, the New Issue page URL can be suffixed with `?title=Issue+Title&body=Issue+Text` and the form will be populated with those strings. Those strings will be used instead of the template if there is one. diff --git a/services/pull/merge.go b/services/pull/merge.go index eb5adc731d2bd..13ac8bf0a7b60 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -84,7 +84,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, closeIssueIndexes := make([]string, 0, len(refs)) for _, ref := range refs { if ref.RefAction == references.XRefActionCloses { - closeIssueIndexes = append(closeIssueIndexes, fmt.Sprintf("%s%d", issueReference, ref.Issue.Index)) + closeIssueIndexes = append(closeIssueIndexes, fmt.Sprintf("close %s%d", issueReference, ref.Issue.Index)) } } if len(closeIssueIndexes) > 0 { From 6c15fb6429e9d7bde1ccfc4526b7e2ff1ae42009 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 16 Feb 2022 15:51:05 +0800 Subject: [PATCH 08/28] Fix test --- services/pull/pull_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index 796c873e6d6db..4a5e3b0715ac1 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -12,7 +12,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" - user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "github.com/stretchr/testify/assert" ) @@ -61,8 +60,7 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { ExternalTrackerFormat: "https://someurl.com/{user}/{repo}/{issue}", }, } - baseRepo := &repo_model.Repository{Name: "testRepo", ID: 1} - baseRepo.Owner = &user_model.User{Name: "testOwner"} + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}).(*repo_model.Repository) baseRepo.Units = []*repo_model.RepoUnit{&externalTracker} pr := unittest.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 2, BaseRepo: baseRepo}).(*models.PullRequest) @@ -76,5 +74,7 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { pr.BaseRepoID = 1 pr.HeadRepoID = 2 - assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) + pr.BaseRepo = nil + pr.HeadRepo = nil + assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo2:branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) } From 2bb16a08b96a73ca5f441bfb9e579dd4ab1374ae Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Mar 2022 00:09:37 +0800 Subject: [PATCH 09/28] Fix and docs --- docs/content/doc/usage/issue-pull-request-templates.en-us.md | 4 ++-- services/pull/merge.go | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/content/doc/usage/issue-pull-request-templates.en-us.md b/docs/content/doc/usage/issue-pull-request-templates.en-us.md index a179c7184bdd7..86621b5d614ab 100644 --- a/docs/content/doc/usage/issue-pull-request-templates.en-us.md +++ b/docs/content/doc/usage/issue-pull-request-templates.en-us.md @@ -52,7 +52,7 @@ Possible file names for PR default merge message templates: - `.gitea/default_merge_message/MANUALLY-MERGED_TEMPLATE.md` - `.gitea/default_merge_message/REBASE-UPDATE-ONLY_TEMPLATE.md` -You can use some variables in these template with `{}` +You can use the following variables enclosed in `{}` inside these templates: - BaseRepoOwner: Base repository owner name of this pull request - BaseRepoName: Base repository name of this pull request @@ -65,7 +65,7 @@ You can use some variables in these template with `{}` - PullRequestPoster: Pull request's poster name - PullRequestIndex: Pull request's index number - IssueReferenceChar: return # if it's internal tracker or ! for external tracker -- ClosedIssueIndexes: return a string contains all issues which will be closed by this pull request i.e. `close #1, close #2` +- ClosingIssues: return a string contains all issues which will be closed by this pull request i.e. `close #1, close #2` Additionally, the New Issue page URL can be suffixed with `?title=Issue+Title&body=Issue+Text` and the form will be populated with those strings. Those strings will be used instead of the template if there is one. diff --git a/services/pull/merge.go b/services/pull/merge.go index 13ac8bf0a7b60..3cb6be6eb9673 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -88,7 +88,9 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, } } if len(closeIssueIndexes) > 0 { - vars["ClosedIssueIndexes"] = strings.Join(closeIssueIndexes, ", ") + vars["ClosingIssues"] = strings.Join(closeIssueIndexes, ", ") + } else { + vars["ClosingIssues"] = "" } } From aea234d66f585aa618b6ae7c2dd8536ed88ae890 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 21 Mar 2022 09:13:12 +0800 Subject: [PATCH 10/28] Improve codes --- go.mod | 1 + go.sum | 2 ++ modules/git/commit.go | 22 ++++++++++--- services/pull/merge.go | 65 ++++++++++++++++++++++---------------- services/pull/pull_test.go | 1 + 5 files changed, 58 insertions(+), 33 deletions(-) diff --git a/go.mod b/go.mod index bfb87a1b37a8b..9dfcba8282a6d 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 gitea.com/lunny/levelqueue v0.4.1 + gitea.com/lunny/size v0.0.0-20190626083611-40ea84d72f67 github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121 github.com/NYTimes/gziphandler v1.1.1 github.com/PuerkitoBio/goquery v1.8.0 diff --git a/go.sum b/go.sum index d969c26bf533d..b76f19284ffa8 100644 --- a/go.sum +++ b/go.sum @@ -80,6 +80,8 @@ gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 h1:tJQRXgZigkLeeW9LP gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8/go.mod h1:fc/pjt5EqNKgqQXYzcas1Z5L5whkZHyOvTA7OzWVJck= gitea.com/lunny/levelqueue v0.4.1 h1:RZ+AFx5gBsZuyqCvofhAkPQ9uaVDPJnsULoJZIYaJNw= gitea.com/lunny/levelqueue v0.4.1/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU= +gitea.com/lunny/size v0.0.0-20190626083611-40ea84d72f67 h1:wUUfZUASTdd6PdI+Bclx3emADj3V0xfsHQlC/a2Wn6A= +gitea.com/lunny/size v0.0.0-20190626083611-40ea84d72f67/go.mod h1:2JbuirmYWI3mHsxDCkmsTNC9kZ44aYX/2cspOKkZTAE= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a/go.mod h1:EXuID2Zs0pAQhH8yz+DNjUbjppKQzKFAn28TMYPB6IU= github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121 h1:r3qt8PCHnfjOv9PN3H+XXKmDA1dfFMIN1AislhlA/ps= diff --git a/modules/git/commit.go b/modules/git/commit.go index 6e73404dc237d..8c194ef502946 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -17,6 +17,7 @@ import ( "strings" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) // Commit represents a git commit. @@ -307,21 +308,32 @@ func (c *Commit) HasFile(filename string) (bool, error) { } // GetFileContent reads a file content as a string or returns false if this was not possible -func (c *Commit) GetFileContent(filename string) (string, bool) { +func (c *Commit) GetFileContent(filename string, limit int) (string, error) { entry, err := c.GetTreeEntryByPath(filename) if err != nil { - return "", false + return "", err } + r, err := entry.Blob().DataAsync() if err != nil { - return "", false + return "", err } defer r.Close() + + if limit > 0 { + bs := make([]byte, limit) + n, err := util.ReadAtMost(r, bs) + if err != nil { + return "", err + } + return string(bs[:n]), nil + } + bytes, err := io.ReadAll(r) if err != nil { - return "", false + return "", err } - return string(bytes), true + return string(bytes), nil } // GetSubModules get all the sub modules of current revision git tree diff --git a/services/pull/merge.go b/services/pull/merge.go index 3cb6be6eb9673..178203f3f80b5 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -33,6 +33,7 @@ import ( asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" + "gitea.com/lunny/size" "github.com/unknwon/com" ) @@ -64,38 +65,46 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, log.Error("GetBranchCommit: %v", err) return "" } - templateContent, ok := commit.GetFileContent(templateFilepath) - if ok { - vars := map[string]string{ - "BaseRepoOwner": pr.BaseRepo.OwnerName, - "BaseRepoName": pr.BaseRepo.Name, - "BaseBranch": pr.BaseBranch, - "HeadRepoOwner": pr.HeadRepo.OwnerName, - "HeadRepoName": pr.HeadRepo.Name, - "HeadBranch": pr.HeadBranch, - "PullRequestTitle": pr.Issue.Title, - "PullRequestDescription": pr.Issue.Content, - "PullRequestPoster": pr.Issue.Poster.Name, - "PullRequestIndex": strconv.FormatInt(pr.Index, 10), - "IssueReferenceChar": issueReference, + templateContent, err := commit.GetFileContent(templateFilepath, int(10*size.K)) + if err != nil { + log.Error("GetFileContent: %v", err) + return "" + } + + vars := map[string]string{ + "BaseRepoOwnerName": pr.BaseRepo.OwnerName, + "BaseRepoName": pr.BaseRepo.Name, + "BaseBranch": pr.BaseBranch, + "HeadRepoOwnerName": pr.HeadRepo.OwnerName, + "HeadRepoName": pr.HeadRepo.Name, + "HeadBranch": pr.HeadBranch, + "PullRequestTitle": pr.Issue.Title, + "PullRequestDescription": pr.Issue.Content, + "PullRequestPosterName": pr.Issue.Poster.Name, + "PullRequestIndex": strconv.FormatInt(pr.Index, 10), + "PullRequestReference": fmt.Sprintf("%s%d", issueReference, pr.Index), + "IssueReferenceChar": issueReference, + } + refs, err := pr.ResolveCrossReferences() + if err == nil { + closeIssueIndexes := make([]string, 0, len(refs)) + closeWord := "close" + if len(setting.Repository.PullRequest.CloseKeywords) > 0 { + closeWord = setting.Repository.PullRequest.CloseKeywords[0] } - refs, err := pr.ResolveCrossReferences() - if err == nil { - closeIssueIndexes := make([]string, 0, len(refs)) - for _, ref := range refs { - if ref.RefAction == references.XRefActionCloses { - closeIssueIndexes = append(closeIssueIndexes, fmt.Sprintf("close %s%d", issueReference, ref.Issue.Index)) - } - } - if len(closeIssueIndexes) > 0 { - vars["ClosingIssues"] = strings.Join(closeIssueIndexes, ", ") - } else { - vars["ClosingIssues"] = "" + for _, ref := range refs { + if ref.RefAction == references.XRefActionCloses { + closeIssueIndexes = append(closeIssueIndexes, fmt.Sprintf("%s %s%d", closeWord, issueReference, ref.Issue.Index)) } } - - return com.Expand(templateContent, vars) + if len(closeIssueIndexes) > 0 { + vars["ClosingIssues"] = strings.Join(closeIssueIndexes, ", ") + } else { + vars["ClosingIssues"] = "" + } } + + return com.Expand(templateContent, vars) } // Squash merge has a different from other styles. diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index 4a5e3b0715ac1..aa138bd9983f9 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" + "github.com/stretchr/testify/assert" ) From 9d81824e4706257bf4eea2a6bc757ba009659dbc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 21 Mar 2022 13:22:37 +0800 Subject: [PATCH 11/28] Update docs and remove unnecessary variables --- .../doc/usage/issue-pull-request-templates.en-us.md | 8 ++++---- go.mod | 1 - go.sum | 2 -- services/pull/merge.go | 4 +--- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/docs/content/doc/usage/issue-pull-request-templates.en-us.md b/docs/content/doc/usage/issue-pull-request-templates.en-us.md index 86621b5d614ab..972cbab956d56 100644 --- a/docs/content/doc/usage/issue-pull-request-templates.en-us.md +++ b/docs/content/doc/usage/issue-pull-request-templates.en-us.md @@ -54,17 +54,17 @@ Possible file names for PR default merge message templates: You can use the following variables enclosed in `{}` inside these templates: -- BaseRepoOwner: Base repository owner name of this pull request +- BaseRepoOwnerName: Base repository owner name of this pull request - BaseRepoName: Base repository name of this pull request - BaseBranch: Base repository target branch name of this pull request -- HeadRepoOwner: Head repository owner name of this pull request +- HeadRepoOwnerName: Head repository owner name of this pull request - HeadRepoName: Head repository name of this pull request - HeadBranch: Head repository branch name of this pull request - PullRequestTitle: Pull request's title - PullRequestDescription: Pull request's description -- PullRequestPoster: Pull request's poster name +- PullRequestPosterName: Pull request's poster name - PullRequestIndex: Pull request's index number -- IssueReferenceChar: return # if it's internal tracker or ! for external tracker +- PullRequestReference: Pull request's reference char with index number. i.e. #1, !2 - ClosingIssues: return a string contains all issues which will be closed by this pull request i.e. `close #1, close #2` Additionally, the New Issue page URL can be suffixed with `?title=Issue+Title&body=Issue+Text` and the form will be populated with those strings. Those strings will be used instead of the template if there is one. diff --git a/go.mod b/go.mod index 9dfcba8282a6d..bfb87a1b37a8b 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 gitea.com/lunny/levelqueue v0.4.1 - gitea.com/lunny/size v0.0.0-20190626083611-40ea84d72f67 github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121 github.com/NYTimes/gziphandler v1.1.1 github.com/PuerkitoBio/goquery v1.8.0 diff --git a/go.sum b/go.sum index b76f19284ffa8..d969c26bf533d 100644 --- a/go.sum +++ b/go.sum @@ -80,8 +80,6 @@ gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 h1:tJQRXgZigkLeeW9LP gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8/go.mod h1:fc/pjt5EqNKgqQXYzcas1Z5L5whkZHyOvTA7OzWVJck= gitea.com/lunny/levelqueue v0.4.1 h1:RZ+AFx5gBsZuyqCvofhAkPQ9uaVDPJnsULoJZIYaJNw= gitea.com/lunny/levelqueue v0.4.1/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU= -gitea.com/lunny/size v0.0.0-20190626083611-40ea84d72f67 h1:wUUfZUASTdd6PdI+Bclx3emADj3V0xfsHQlC/a2Wn6A= -gitea.com/lunny/size v0.0.0-20190626083611-40ea84d72f67/go.mod h1:2JbuirmYWI3mHsxDCkmsTNC9kZ44aYX/2cspOKkZTAE= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a/go.mod h1:EXuID2Zs0pAQhH8yz+DNjUbjppKQzKFAn28TMYPB6IU= github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121 h1:r3qt8PCHnfjOv9PN3H+XXKmDA1dfFMIN1AislhlA/ps= diff --git a/services/pull/merge.go b/services/pull/merge.go index 178203f3f80b5..667637cb34afc 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -33,7 +33,6 @@ import ( asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" - "gitea.com/lunny/size" "github.com/unknwon/com" ) @@ -65,7 +64,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, log.Error("GetBranchCommit: %v", err) return "" } - templateContent, err := commit.GetFileContent(templateFilepath, int(10*size.K)) + templateContent, err := commit.GetFileContent(templateFilepath, 10*1024) if err != nil { log.Error("GetFileContent: %v", err) return "" @@ -83,7 +82,6 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, "PullRequestPosterName": pr.Issue.Poster.Name, "PullRequestIndex": strconv.FormatInt(pr.Index, 10), "PullRequestReference": fmt.Sprintf("%s%d", issueReference, pr.Index), - "IssueReferenceChar": issueReference, } refs, err := pr.ResolveCrossReferences() if err == nil { From 86a9b44ca896a7a39aa9676f0f4f70092f2a64f8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 21 Mar 2022 19:08:20 +0800 Subject: [PATCH 12/28] return error for GetDefaultMergeMessage --- routers/api/v1/repo/pull.go | 6 ++- routers/web/repo/issue.go | 15 ++++++- routers/web/repo/pull.go | 6 ++- services/pull/merge.go | 87 ++++++++++++++++++------------------- services/pull/pull_test.go | 19 ++++++-- 5 files changed, 80 insertions(+), 53 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 5e0cc50c24f00..5536348da6fee 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -829,7 +829,11 @@ func MergePullRequest(ctx *context.APIContext) { message := strings.TrimSpace(form.MergeTitleField) if len(message) == 0 { - message = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pr, repo_model.MergeStyle(form.Do)) + message, err = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pr, repo_model.MergeStyle(form.Do)) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetDefaultMergeMessage", err) + return + } } form.MergeMessageField = strings.TrimSpace(form.MergeMessageField) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 5c6caaa5acb71..7ddeb05f719cf 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1594,8 +1594,19 @@ func ViewIssue(ctx *context.Context) { ctx.Data["MergeStyle"] = mergeStyle - ctx.Data["DefaultMergeMessage"] = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pull, mergeStyle) - ctx.Data["DefaultSquashMergeMessage"] = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pull, repo_model.MergeStyleSquash) + defaultMergeMessage, err := pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pull, mergeStyle) + if err != nil { + ctx.ServerError("GetDefaultMergeMessage", err) + return + } + ctx.Data["DefaultMergeMessage"] = defaultMergeMessage + + defaultSquashMergeMessage, err := pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pull, repo_model.MergeStyleSquash) + if err != nil { + ctx.ServerError("GetDefaultSquashMergeMessage", err) + return + } + ctx.Data["DefaultSquashMergeMessage"] = defaultSquashMergeMessage if err = pull.LoadProtectedBranch(); err != nil { ctx.ServerError("LoadProtectedBranch", err) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 089dfb4ea8053..73131bb6ae19f 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -958,7 +958,11 @@ func MergePullRequest(ctx *context.Context) { message := strings.TrimSpace(form.MergeTitleField) if len(message) == 0 { - message = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pr, repo_model.MergeStyle(form.Do)) + message, err = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pr, repo_model.MergeStyle(form.Do)) + if err != nil { + ctx.ServerError("GetDefaultMergeMessage", err) + return + } } form.MergeMessageField = strings.TrimSpace(form.MergeMessageField) diff --git a/services/pull/merge.go b/services/pull/merge.go index 667637cb34afc..38e3b7a88c8e0 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -37,18 +37,15 @@ import ( ) // GetDefaultMergeMessage returns default message used when merging pull request -func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, mergeStyle repo_model.MergeStyle) string { +func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, mergeStyle repo_model.MergeStyle) (string, error) { if err := pr.LoadHeadRepo(); err != nil { - log.Error("LoadHeadRepo[%d]: %v", pr.HeadRepoID, err) - return "" + return "", err } if err := pr.LoadIssue(); err != nil { - log.Error("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err) - return "" + return "", err } if err := pr.LoadBaseRepo(); err != nil { - log.Error("LoadBaseRepo: %v", err) - return "" + return "", err } isExternalTracker := pr.BaseRepo.UnitEnabled(unit.TypeExternalTracker) @@ -61,60 +58,60 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, templateFilepath := fmt.Sprintf(".gitea/default_merge_message/%s_TEMPLATE.md", strings.ToUpper(string(mergeStyle))) commit, err := baseGitRepo.GetBranchCommit(pr.BaseRepo.DefaultBranch) if err != nil { - log.Error("GetBranchCommit: %v", err) - return "" + return "", err } templateContent, err := commit.GetFileContent(templateFilepath, 10*1024) if err != nil { - log.Error("GetFileContent: %v", err) - return "" - } - - vars := map[string]string{ - "BaseRepoOwnerName": pr.BaseRepo.OwnerName, - "BaseRepoName": pr.BaseRepo.Name, - "BaseBranch": pr.BaseBranch, - "HeadRepoOwnerName": pr.HeadRepo.OwnerName, - "HeadRepoName": pr.HeadRepo.Name, - "HeadBranch": pr.HeadBranch, - "PullRequestTitle": pr.Issue.Title, - "PullRequestDescription": pr.Issue.Content, - "PullRequestPosterName": pr.Issue.Poster.Name, - "PullRequestIndex": strconv.FormatInt(pr.Index, 10), - "PullRequestReference": fmt.Sprintf("%s%d", issueReference, pr.Index), - } - refs, err := pr.ResolveCrossReferences() - if err == nil { - closeIssueIndexes := make([]string, 0, len(refs)) - closeWord := "close" - if len(setting.Repository.PullRequest.CloseKeywords) > 0 { - closeWord = setting.Repository.PullRequest.CloseKeywords[0] + if !git.IsErrNotExist(err) { + return "", err } - for _, ref := range refs { - if ref.RefAction == references.XRefActionCloses { - closeIssueIndexes = append(closeIssueIndexes, fmt.Sprintf("%s %s%d", closeWord, issueReference, ref.Issue.Index)) - } + } else { + vars := map[string]string{ + "BaseRepoOwnerName": pr.BaseRepo.OwnerName, + "BaseRepoName": pr.BaseRepo.Name, + "BaseBranch": pr.BaseBranch, + "HeadRepoOwnerName": pr.HeadRepo.OwnerName, + "HeadRepoName": pr.HeadRepo.Name, + "HeadBranch": pr.HeadBranch, + "PullRequestTitle": pr.Issue.Title, + "PullRequestDescription": pr.Issue.Content, + "PullRequestPosterName": pr.Issue.Poster.Name, + "PullRequestIndex": strconv.FormatInt(pr.Index, 10), + "PullRequestReference": fmt.Sprintf("%s%d", issueReference, pr.Index), } - if len(closeIssueIndexes) > 0 { - vars["ClosingIssues"] = strings.Join(closeIssueIndexes, ", ") - } else { - vars["ClosingIssues"] = "" + refs, err := pr.ResolveCrossReferences() + if err == nil { + closeIssueIndexes := make([]string, 0, len(refs)) + closeWord := "close" + if len(setting.Repository.PullRequest.CloseKeywords) > 0 { + closeWord = setting.Repository.PullRequest.CloseKeywords[0] + } + for _, ref := range refs { + if ref.RefAction == references.XRefActionCloses { + closeIssueIndexes = append(closeIssueIndexes, fmt.Sprintf("%s %s%d", closeWord, issueReference, ref.Issue.Index)) + } + } + if len(closeIssueIndexes) > 0 { + vars["ClosingIssues"] = strings.Join(closeIssueIndexes, ", ") + } else { + vars["ClosingIssues"] = "" + } } - } - return com.Expand(templateContent, vars) + return com.Expand(templateContent, vars), nil + } } // Squash merge has a different from other styles. if mergeStyle == repo_model.MergeStyleSquash { - return fmt.Sprintf("%s (%s%d)", pr.Issue.Title, issueReference, pr.Issue.Index) + return fmt.Sprintf("%s (%s%d)", pr.Issue.Title, issueReference, pr.Issue.Index), nil } if pr.BaseRepoID == pr.HeadRepoID { - return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch) + return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), nil } - return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch) + return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch), nil } // Merge merges pull request to base repository. diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index aa138bd9983f9..dbf5a4fed5e36 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -45,11 +45,16 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { assert.NoError(t, err) defer gitRepo.Close() - assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) + mergeMessage, err := GetDefaultMergeMessage(gitRepo, pr, "") + assert.NoError(t, err) + + assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", mergeMessage) pr.BaseRepoID = 1 pr.HeadRepoID = 2 - assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) + mergeMessage, err = GetDefaultMergeMessage(gitRepo, pr, "") + + assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", mergeMessage) } func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { @@ -71,11 +76,17 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { assert.NoError(t, err) defer gitRepo.Close() - assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) + mergeMessage, err := GetDefaultMergeMessage(gitRepo, pr, "") + assert.NoError(t, err) + + assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", mergeMessage) pr.BaseRepoID = 1 pr.HeadRepoID = 2 pr.BaseRepo = nil pr.HeadRepo = nil - assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo2:branch2 into master", GetDefaultMergeMessage(gitRepo, pr, "")) + mergeMessage, err = GetDefaultMergeMessage(gitRepo, pr, "") + assert.NoError(t, err) + + assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo2:branch2 into master", mergeMessage) } From 515f766cc4d7e5363cbdb0b901470c0ac16f9340 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 21 Mar 2022 19:15:13 +0800 Subject: [PATCH 13/28] Fix test --- services/pull/pull_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index dbf5a4fed5e36..878ba3235a947 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -47,13 +47,12 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { mergeMessage, err := GetDefaultMergeMessage(gitRepo, pr, "") assert.NoError(t, err) - assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", mergeMessage) pr.BaseRepoID = 1 pr.HeadRepoID = 2 mergeMessage, err = GetDefaultMergeMessage(gitRepo, pr, "") - + assert.NoError(t, err) assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", mergeMessage) } From f7c0c99fd84545b24ad34fc5f88ad805fc9cee0f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 7 May 2022 10:02:36 +0800 Subject: [PATCH 14/28] improve code --- .../issue-pull-request-templates.en-us.md | 2 +- integrations/pull_merge_test.go | 7 +-- models/pull.go | 43 --------------- models/pull_test.go | 53 ------------------- routers/api/v1/repo/pull.go | 6 --- routers/web/repo/pull.go | 17 +----- services/forms/repo_form.go | 26 --------- services/pull/merge.go | 26 ++++----- services/pull/pull_test.go | 4 +- templates/repo/issue/view_content/pull.tmpl | 6 +-- 10 files changed, 25 insertions(+), 165 deletions(-) diff --git a/docs/content/doc/usage/issue-pull-request-templates.en-us.md b/docs/content/doc/usage/issue-pull-request-templates.en-us.md index 972cbab956d56..185b895d1070e 100644 --- a/docs/content/doc/usage/issue-pull-request-templates.en-us.md +++ b/docs/content/doc/usage/issue-pull-request-templates.en-us.md @@ -52,7 +52,7 @@ Possible file names for PR default merge message templates: - `.gitea/default_merge_message/MANUALLY-MERGED_TEMPLATE.md` - `.gitea/default_merge_message/REBASE-UPDATE-ONLY_TEMPLATE.md` -You can use the following variables enclosed in `{}` inside these templates: +You can use the following variables enclosed in `${}` inside these templates which follow [os.Expand](https://pkg.go.dev/os#Expand) syntax: - BaseRepoOwnerName: Base repository owner name of this pull request - BaseRepoName: Base repository name of this pull request diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go index 476f7ab52f9f8..c50913383c08b 100644 --- a/integrations/pull_merge_test.go +++ b/integrations/pull_merge_test.go @@ -6,6 +6,7 @@ package integrations import ( "bytes" + "context" "fmt" "net/http" "net/http/httptest" @@ -243,11 +244,11 @@ func TestCantMergeConflict(t *testing.T) { gitRepo, err := git.OpenRepository(git.DefaultContext, repo_model.RepoPath(user1.Name, repo1.Name)) assert.NoError(t, err) - err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT") + err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT") assert.Error(t, err, "Merge should return an error due to conflict") assert.True(t, models.IsErrMergeConflicts(err), "Merge error is not a conflict error") - err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT") + err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT") assert.Error(t, err, "Merge should return an error due to conflict") assert.True(t, models.IsErrRebaseConflicts(err), "Merge error is not a conflict error") gitRepo.Close() @@ -342,7 +343,7 @@ func TestCantMergeUnrelated(t *testing.T) { BaseBranch: "base", }).(*models.PullRequest) - err = pull.Merge(pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED") + err = pull.Merge(context.Background(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED") assert.Error(t, err, "Merge should return an error due to unrelated") assert.True(t, models.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error") gitRepo.Close() diff --git a/models/pull.go b/models/pull.go index 0fa3bdf14f72a..fc5c0d61b330a 100644 --- a/models/pull.go +++ b/models/pull.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -228,34 +227,6 @@ func (pr *PullRequest) LoadProtectedBranchCtx(ctx context.Context) (err error) { return } -// GetDefaultMergeMessage returns default message used when merging pull request -func (pr *PullRequest) GetDefaultMergeMessage(ctx context.Context) (string, error) { - if pr.HeadRepo == nil { - var err error - pr.HeadRepo, err = repo_model.GetRepositoryByIDCtx(ctx, pr.HeadRepoID) - if err != nil { - return "", fmt.Errorf("GetRepositoryById[%d]: %v", pr.HeadRepoID, err) - } - } - if err := pr.LoadIssueCtx(ctx); err != nil { - return "", fmt.Errorf("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err) - } - if err := pr.LoadBaseRepoCtx(ctx); err != nil { - return "", fmt.Errorf("LoadBaseRepo: %v", err) - } - - issueReference := "#" - if pr.BaseRepo.UnitEnabledCtx(ctx, unit.TypeExternalTracker) { - issueReference = "!" - } - - if pr.BaseRepoID == pr.HeadRepoID { - return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), nil - } - - return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch), nil -} - // ReviewCount represents a count of Reviews type ReviewCount struct { IssueID int64 @@ -338,20 +309,6 @@ func (pr *PullRequest) getReviewedByLines(writer io.Writer) error { return committer.Commit() } -// GetDefaultSquashMessage returns default message used when squash and merging pull request -func (pr *PullRequest) GetDefaultSquashMessage(ctx context.Context) (string, error) { - if err := pr.LoadIssueCtx(ctx); err != nil { - return "", fmt.Errorf("LoadIssue: %v", err) - } - if err := pr.LoadBaseRepoCtx(ctx); err != nil { - return "", fmt.Errorf("LoadBaseRepo: %v", err) - } - if pr.BaseRepo.UnitEnabledCtx(ctx, unit.TypeExternalTracker) { - return fmt.Sprintf("%s (!%d)", pr.Issue.Title, pr.Issue.Index), nil - } - return fmt.Sprintf("%s (#%d)", pr.Issue.Title, pr.Issue.Index), nil -} - // GetGitRefName returns git ref for hidden pull request branch func (pr *PullRequest) GetGitRefName() string { return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index) diff --git a/models/pull_test.go b/models/pull_test.go index 92cf9a6524171..6119bca692769 100644 --- a/models/pull_test.go +++ b/models/pull_test.go @@ -8,10 +8,7 @@ import ( "testing" "code.gitea.io/gitea/models/db" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" - user_model "code.gitea.io/gitea/models/user" "github.com/stretchr/testify/assert" ) @@ -256,53 +253,3 @@ func TestPullRequest_GetWorkInProgressPrefixWorkInProgress(t *testing.T) { pr.Issue.Title = "[wip] " + original assert.Equal(t, "[wip]", pr.GetWorkInProgressPrefix()) } - -func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest) - - msg, err := pr.GetDefaultMergeMessage(db.DefaultContext) - assert.NoError(t, err) - assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", msg) - - pr.BaseRepoID = 1 - pr.HeadRepoID = 2 - msg, err = pr.GetDefaultMergeMessage(db.DefaultContext) - assert.NoError(t, err) - assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", msg) -} - -func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - externalTracker := repo_model.RepoUnit{ - Type: unit.TypeExternalTracker, - Config: &repo_model.ExternalTrackerConfig{ - ExternalTrackerFormat: "https://someurl.com/{user}/{repo}/{issue}", - }, - } - baseRepo := &repo_model.Repository{Name: "testRepo", ID: 1} - baseRepo.Owner = &user_model.User{Name: "testOwner"} - baseRepo.Units = []*repo_model.RepoUnit{&externalTracker} - - pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2, BaseRepo: baseRepo}).(*PullRequest) - - msg, err := pr.GetDefaultMergeMessage(db.DefaultContext) - assert.NoError(t, err) - assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", msg) - - pr.BaseRepoID = 1 - pr.HeadRepoID = 2 - msg, err = pr.GetDefaultMergeMessage(db.DefaultContext) - assert.NoError(t, err) - assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", msg) -} - -func TestPullRequest_GetDefaultSquashMessage(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - pr := unittest.AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest) - - msg, err := pr.GetDefaultSquashMessage(db.DefaultContext) - assert.NoError(t, err) - assert.Equal(t, "issue3 (#3)", msg) -} diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 5536348da6fee..874e2205d58e9 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -801,12 +801,6 @@ func MergePullRequest(ctx *context.APIContext) { return } - // set defaults to propagate needed fields - if err := form.SetDefaults(ctx, pr); err != nil { - ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err)) - return - } - if form.MergeWhenChecksSucceed { scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), form.MergeTitleField) if err != nil { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 73131bb6ae19f..27b61309a5756 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -950,14 +950,9 @@ func MergePullRequest(ctx *context.Context) { return } - // set defaults to propagate needed fields - if err := form.SetDefaults(ctx, pr); err != nil { - ctx.ServerError("SetDefaults", fmt.Errorf("SetDefaults: %v", err)) - return - } - message := strings.TrimSpace(form.MergeTitleField) if len(message) == 0 { + var err error message, err = pull_service.GetDefaultMergeMessage(ctx.Repo.GitRepo, pr, repo_model.MergeStyle(form.Do)) if err != nil { ctx.ServerError("GetDefaultMergeMessage", err) @@ -970,15 +965,7 @@ func MergePullRequest(ctx *context.Context) { message += "\n\n" + form.MergeMessageField } - pr.Issue = issue - pr.Issue.Repo = ctx.Repo.Repository - - noDeps, err := models.IssueNoDependenciesLeft(ctx, issue) - if err != nil { - return - } - - if err := pull_service.Merge(pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, form.MergeTitleField); err != nil { + if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) ctx.Redirect(issue.Link()) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index bacee9a13ca7b..6c61ed00ecb8d 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -6,7 +6,6 @@ package forms import ( - stdContext "context" "net/http" "net/url" "strings" @@ -602,31 +601,6 @@ func (f *MergePullRequestForm) Validate(req *http.Request, errs binding.Errors) return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } -// SetDefaults if not provided for mergestyle and commit message -func (f *MergePullRequestForm) SetDefaults(ctx stdContext.Context, pr *models.PullRequest) (err error) { - if f.Do == "" { - f.Do = "merge" - } - - f.MergeTitleField = strings.TrimSpace(f.MergeTitleField) - if len(f.MergeTitleField) == 0 { - switch f.Do { - case "merge", "rebase-merge": - f.MergeTitleField, err = pr.GetDefaultMergeMessage(ctx) - case "squash": - f.MergeTitleField, err = pr.GetDefaultSquashMessage(ctx) - } - } - - f.MergeMessageField = strings.TrimSpace(f.MergeMessageField) - if len(f.MergeMessageField) > 0 { - f.MergeTitleField += "\n\n" + f.MergeMessageField - f.MergeMessageField = "" - } - - return -} - // CodeCommentForm form for adding code comments for PRs type CodeCommentForm struct { Origin string `binding:"Required;In(timeline,diff)"` diff --git a/services/pull/merge.go b/services/pull/merge.go index 38e3b7a88c8e0..535191bb34e50 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -32,8 +32,6 @@ import ( "code.gitea.io/gitea/modules/timeutil" asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" - - "github.com/unknwon/com" ) // GetDefaultMergeMessage returns default message used when merging pull request @@ -60,7 +58,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, if err != nil { return "", err } - templateContent, err := commit.GetFileContent(templateFilepath, 10*1024) + templateContent, err := commit.GetFileContent(templateFilepath, setting.Repository.PullRequest.DefaultMergeMessageSize) if err != nil { if !git.IsErrNotExist(err) { return "", err @@ -79,7 +77,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, "PullRequestIndex": strconv.FormatInt(pr.Index, 10), "PullRequestReference": fmt.Sprintf("%s%d", issueReference, pr.Index), } - refs, err := pr.ResolveCrossReferences() + refs, err := pr.ResolveCrossReferences(context.Background()) if err == nil { closeIssueIndexes := make([]string, 0, len(refs)) closeWord := "close" @@ -98,7 +96,9 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, } } - return com.Expand(templateContent, vars), nil + return os.Expand(templateContent, func(s string) string { + return vars[s] + }), nil } } @@ -116,7 +116,7 @@ func GetDefaultMergeMessage(baseGitRepo *git.Repository, pr *models.PullRequest, // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) -func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) error { +func Merge(ctx context.Context, pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) error { if err := pr.LoadHeadRepo(); err != nil { log.Error("LoadHeadRepo: %v", err) return fmt.Errorf("LoadHeadRepo: %v", err) @@ -160,18 +160,18 @@ func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repos pr.Merger = doer pr.MergerID = doer.ID - if _, err := pr.SetMerged(db.DefaultContext); err != nil { + if _, err := pr.SetMerged(ctx); err != nil { log.Error("setMerged [%d]: %v", pr.ID, err) } - if err := pr.LoadIssueCtx(db.DefaultContext); err != nil { + if err := pr.LoadIssueCtx(ctx); err != nil { log.Error("loadIssue [%d]: %v", pr.ID, err) } - if err := pr.Issue.LoadRepo(db.DefaultContext); err != nil { + if err := pr.Issue.LoadRepo(ctx); err != nil { log.Error("loadRepo for issue [%d]: %v", pr.ID, err) } - if err := pr.Issue.Repo.GetOwner(db.DefaultContext); err != nil { + if err := pr.Issue.Repo.GetOwner(ctx); err != nil { log.Error("GetOwner for issue repo [%d]: %v", pr.ID, err) } @@ -181,17 +181,17 @@ func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repos cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) // Resolve cross references - refs, err := pr.ResolveCrossReferences(db.DefaultContext) + refs, err := pr.ResolveCrossReferences(ctx) if err != nil { log.Error("ResolveCrossReferences: %v", err) return nil } for _, ref := range refs { - if err = ref.LoadIssueCtx(db.DefaultContext); err != nil { + if err = ref.LoadIssueCtx(ctx); err != nil { return err } - if err = ref.Issue.LoadRepo(db.DefaultContext); err != nil { + if err = ref.Issue.LoadRepo(ctx); err != nil { return err } close := ref.RefAction == references.XRefActionCloses diff --git a/services/pull/pull_test.go b/services/pull/pull_test.go index 878ba3235a947..09bae97780929 100644 --- a/services/pull/pull_test.go +++ b/services/pull/pull_test.go @@ -41,7 +41,7 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { pr := unittest.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 2}).(*models.PullRequest) assert.NoError(t, pr.LoadBaseRepo()) - gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) + gitRepo, err := git.OpenRepository(git.DefaultContext, pr.BaseRepo.RepoPath()) assert.NoError(t, err) defer gitRepo.Close() @@ -71,7 +71,7 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { pr := unittest.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 2, BaseRepo: baseRepo}).(*models.PullRequest) assert.NoError(t, pr.LoadBaseRepo()) - gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) + gitRepo, err := git.OpenRepository(git.DefaultContext, pr.BaseRepo.RepoPath()) assert.NoError(t, err) defer gitRepo.Close() diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index e5519617cabf8..07b2e89d2432c 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -329,7 +329,7 @@ {{.CsrfTokenHtml}}
- +
@@ -375,7 +375,7 @@ {{.CsrfTokenHtml}}
- +