From 271db6ff226f0747ce45e16e296b41ec972dc892 Mon Sep 17 00:00:00 2001 From: Caesar Schinas Date: Fri, 25 Aug 2023 21:49:17 +0100 Subject: [PATCH 1/2] [FEAT] support `.forgejo` dir for issue and PR templates --- routers/web/repo/issue.go | 6 ++++++ routers/web/repo/pull.go | 6 ++++++ routers/web/repo/view.go | 4 ++++ services/issue/template.go | 4 ++++ services/pull/merge.go | 10 ++++++++-- 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 9da62bb00b..9441034b91 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -77,6 +77,12 @@ var IssueTemplateCandidates = []string{ "issue_template.md", "issue_template.yaml", "issue_template.yml", + ".forgejo/ISSUE_TEMPLATE.md", + ".forgejo/ISSUE_TEMPLATE.yaml", + ".forgejo/ISSUE_TEMPLATE.yml", + ".forgejo/issue_template.md", + ".forgejo/issue_template.yaml", + ".forgejo/issue_template.yml", ".gitea/ISSUE_TEMPLATE.md", ".gitea/ISSUE_TEMPLATE.yaml", ".gitea/ISSUE_TEMPLATE.yml", diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 2544b29398..0ed4fdce9d 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -65,6 +65,12 @@ var pullRequestTemplateCandidates = []string{ "pull_request_template.md", "pull_request_template.yaml", "pull_request_template.yml", + ".forgejo/PULL_REQUEST_TEMPLATE.md", + ".forgejo/PULL_REQUEST_TEMPLATE.yaml", + ".forgejo/PULL_REQUEST_TEMPLATE.yml", + ".forgejo/pull_request_template.md", + ".forgejo/pull_request_template.yaml", + ".forgejo/pull_request_template.yml", ".gitea/PULL_REQUEST_TEMPLATE.md", ".gitea/PULL_REQUEST_TEMPLATE.yaml", ".gitea/PULL_REQUEST_TEMPLATE.yml", diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 1dda3a05bc..9dc708bca4 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -94,6 +94,10 @@ func findReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry, try if entry.Name() == "docs" || docsEntries[0] == nil { docsEntries[0] = entry } + case ".forgejo": + if entry.Name() == ".forgejo" || docsEntries[1] == nil { + docsEntries[1] = entry + } case ".gitea": if entry.Name() == ".gitea" || docsEntries[1] == nil { docsEntries[1] = entry diff --git a/services/issue/template.go b/services/issue/template.go index b6ae077987..47633e5d85 100644 --- a/services/issue/template.go +++ b/services/issue/template.go @@ -23,6 +23,8 @@ import ( var templateDirCandidates = []string{ "ISSUE_TEMPLATE", "issue_template", + ".forgejo/ISSUE_TEMPLATE", + ".forgejo/issue_template", ".gitea/ISSUE_TEMPLATE", ".gitea/issue_template", ".github/ISSUE_TEMPLATE", @@ -32,6 +34,8 @@ var templateDirCandidates = []string{ } var templateConfigCandidates = []string{ + ".forgejo/ISSUE_TEMPLATE/config", + ".forgejo/issue_template/config", ".gitea/ISSUE_TEMPLATE/config", ".gitea/issue_template/config", ".github/ISSUE_TEMPLATE/config", diff --git a/services/pull/merge.go b/services/pull/merge.go index 91b110351a..718e964014 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -55,12 +55,18 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue } if 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 { return "", "", err } - templateContent, err := commit.GetFileContent(templateFilepath, setting.Repository.PullRequest.DefaultMergeMessageSize) + + templateFilepathForgejo := fmt.Sprintf(".forgejo/default_merge_message/%s_TEMPLATE.md", strings.ToUpper(string(mergeStyle))) + templateFilepathGitea := fmt.Sprintf(".gitea/default_merge_message/%s_TEMPLATE.md", strings.ToUpper(string(mergeStyle))) + + templateContent, err := commit.GetFileContent(templateFilepathForgejo, setting.Repository.PullRequest.DefaultMergeMessageSize) + if _, ok := err.(git.ErrNotExist); ok { + templateContent, err = commit.GetFileContent(templateFilepathGitea, setting.Repository.PullRequest.DefaultMergeMessageSize) + } if err != nil { if !git.IsErrNotExist(err) { return "", "", err From 1624ebc83616936d51ea399750a7041e1aea01d2 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 5 Feb 2024 16:33:40 +0100 Subject: [PATCH 2/2] [TESTS] issue & PR templates in `.forgejo` are recognized This adds a few tests for the previous change, to verify that issue template configs, issue templates and pr templates are all recognized in `.forgejo` directories. Signed-off-by: Gergely Nagy --- tests/integration/api_issue_config_test.go | 51 +++++++--- tests/integration/api_issue_templates_test.go | 74 +++++++++++--- tests/integration/pull_create_test.go | 99 +++++++++++++++++++ 3 files changed, 201 insertions(+), 23 deletions(-) diff --git a/tests/integration/api_issue_config_test.go b/tests/integration/api_issue_config_test.go index b9125438b6..d37036381e 100644 --- a/tests/integration/api_issue_config_test.go +++ b/tests/integration/api_issue_config_test.go @@ -1,4 +1,5 @@ // Copyright 2023 The Gitea Authors. All rights reserved. +// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved. // SPDX-License-Identifier: MIT package integration @@ -18,14 +19,18 @@ import ( "gopkg.in/yaml.v3" ) -func createIssueConfig(t *testing.T, user *user_model.User, repo *repo_model.Repository, issueConfig map[string]any) { +func createIssueConfigInDirectory(t *testing.T, user *user_model.User, repo *repo_model.Repository, dir string, issueConfig map[string]any) { config, err := yaml.Marshal(issueConfig) assert.NoError(t, err) - err = createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/config.yaml", repo.DefaultBranch, string(config)) + err = createOrReplaceFileInBranch(user, repo, fmt.Sprintf("%s/ISSUE_TEMPLATE/config.yaml", dir), repo.DefaultBranch, string(config)) assert.NoError(t, err) } +func createIssueConfig(t *testing.T, user *user_model.User, repo *repo_model.Repository, issueConfig map[string]any) { + createIssueConfigInDirectory(t, user, repo, ".gitea", issueConfig) +} + func getIssueConfig(t *testing.T, owner, repo string) api.IssueConfig { urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issue_config", owner, repo) req := NewRequest(t, "GET", urlStr) @@ -44,6 +49,8 @@ func TestAPIRepoGetIssueConfig(t *testing.T) { owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) t.Run("Default", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + issueConfig := getIssueConfig(t, owner.Name, repo.Name) assert.True(t, issueConfig.BlankIssuesEnabled) @@ -51,6 +58,8 @@ func TestAPIRepoGetIssueConfig(t *testing.T) { }) t.Run("DisableBlankIssues", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + config := make(map[string]any) config["blank_issues_enabled"] = false @@ -63,6 +72,8 @@ func TestAPIRepoGetIssueConfig(t *testing.T) { }) t.Run("ContactLinks", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + contactLink := make(map[string]string) contactLink["name"] = "TestName" contactLink["url"] = "https://example.com" @@ -84,6 +95,8 @@ func TestAPIRepoGetIssueConfig(t *testing.T) { }) t.Run("Full", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + contactLink := make(map[string]string) contactLink["name"] = "TestName" contactLink["url"] = "https://example.com" @@ -113,6 +126,8 @@ func TestAPIRepoIssueConfigPaths(t *testing.T) { owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) templateConfigCandidates := []string{ + ".forgejo/ISSUE_TEMPLATE/config", + ".forgejo/issue_template/config", ".gitea/ISSUE_TEMPLATE/config", ".gitea/issue_template/config", ".github/ISSUE_TEMPLATE/config", @@ -123,6 +138,8 @@ func TestAPIRepoIssueConfigPaths(t *testing.T) { for _, extension := range []string{".yaml", ".yml"} { fullPath := canidate + extension t.Run(fullPath, func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + configMap := make(map[string]any) configMap["blank_issues_enabled"] = false @@ -153,6 +170,8 @@ func TestAPIRepoValidateIssueConfig(t *testing.T) { urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issue_config/validate", owner.Name, repo.Name) t.Run("Valid", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", urlStr) resp := MakeRequest(t, req, http.StatusOK) @@ -164,18 +183,28 @@ func TestAPIRepoValidateIssueConfig(t *testing.T) { }) t.Run("Invalid", func(t *testing.T) { - config := make(map[string]any) - config["blank_issues_enabled"] = "Test" + dirs := []string{".gitea", ".forgejo"} + for _, dir := range dirs { + t.Run(dir, func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + defer func() { + deleteFileInBranch(owner, repo, fmt.Sprintf("%s/ISSUE_TEMPLATE/config.yaml", dir), repo.DefaultBranch) + }() - createIssueConfig(t, owner, repo, config) + config := make(map[string]any) + config["blank_issues_enabled"] = "Test" - req := NewRequest(t, "GET", urlStr) - resp := MakeRequest(t, req, http.StatusOK) + createIssueConfigInDirectory(t, owner, repo, dir, config) - var issueConfigValidation api.IssueConfigValidation - DecodeJSON(t, resp, &issueConfigValidation) + req := NewRequest(t, "GET", urlStr) + resp := MakeRequest(t, req, http.StatusOK) - assert.False(t, issueConfigValidation.Valid) - assert.NotEmpty(t, issueConfigValidation.Message) + var issueConfigValidation api.IssueConfigValidation + DecodeJSON(t, resp, &issueConfigValidation) + + assert.False(t, issueConfigValidation.Valid) + assert.NotEmpty(t, issueConfigValidation.Message) + }) + } }) } diff --git a/tests/integration/api_issue_templates_test.go b/tests/integration/api_issue_templates_test.go index 0d3ddb21bd..15c2dd422b 100644 --- a/tests/integration/api_issue_templates_test.go +++ b/tests/integration/api_issue_templates_test.go @@ -34,13 +34,24 @@ func TestAPIIssueTemplateList(t *testing.T) { }) t.Run("existing template", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - defer func() { - deleteFileInBranch(user, repo, "ISSUE_TEMPLATE/test.md", repo.DefaultBranch) - }() + templateCandidates := []string{ + ".forgejo/ISSUE_TEMPLATE/test.md", + ".forgejo/issue_template/test.md", + ".gitea/ISSUE_TEMPLATE/test.md", + ".gitea/issue_template/test.md", + ".github/ISSUE_TEMPLATE/test.md", + ".github/issue_template/test.md", + } - err := createOrReplaceFileInBranch(user, repo, "ISSUE_TEMPLATE/test.md", repo.DefaultBranch, - `--- + for _, template := range templateCandidates { + t.Run(template, func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + defer func() { + deleteFileInBranch(user, repo, template, repo.DefaultBranch) + }() + + err := createOrReplaceFileInBranch(user, repo, template, repo.DefaultBranch, + `--- name: 'Template Name' about: 'This template is for testing!' title: '[TEST] ' @@ -48,17 +59,56 @@ ref: 'main' --- This is the template!`) - assert.NoError(t, err) + assert.NoError(t, err) + + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/issue_templates", repo.FullName())) + resp := MakeRequest(t, req, http.StatusOK) + var issueTemplates []*api.IssueTemplate + DecodeJSON(t, resp, &issueTemplates) + assert.Len(t, issueTemplates, 1) + assert.Equal(t, "Template Name", issueTemplates[0].Name) + assert.Equal(t, "This template is for testing!", issueTemplates[0].About) + assert.Equal(t, "refs/heads/main", issueTemplates[0].Ref) + assert.Equal(t, template, issueTemplates[0].FileName) + }) + } + }) + + t.Run("multiple templates", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + templatePriority := []string{ + ".forgejo/issue_template/test.md", + ".gitea/issue_template/test.md", + ".github/issue_template/test.md", + } + defer func() { + for _, template := range templatePriority { + deleteFileInBranch(user, repo, template, repo.DefaultBranch) + } + }() + + for _, template := range templatePriority { + err := createOrReplaceFileInBranch(user, repo, template, repo.DefaultBranch, + `--- +name: 'Template Name' +about: 'This template is for testing!' +title: '[TEST] ' +ref: 'main' +--- + +This is the template!`) + assert.NoError(t, err) + } req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/issue_templates", repo.FullName())) resp := MakeRequest(t, req, http.StatusOK) var issueTemplates []*api.IssueTemplate DecodeJSON(t, resp, &issueTemplates) - assert.Len(t, issueTemplates, 1) - assert.Equal(t, "Template Name", issueTemplates[0].Name) - assert.Equal(t, "This template is for testing!", issueTemplates[0].About) - assert.Equal(t, "refs/heads/main", issueTemplates[0].Ref) - assert.Equal(t, "ISSUE_TEMPLATE/test.md", issueTemplates[0].FileName) + + // If templates have the same filename and content, but in different + // directories, they count as different templates, and all are + // considered. + assert.Len(t, issueTemplates, 3) }) }) } diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 0aeecd5880..cca0a2e68c 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -96,6 +96,105 @@ func TestPullCreate(t *testing.T) { }) } +func TestPullCreateWithPullTemplate(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + baseUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + forkUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + templateCandidates := []string{ + ".forgejo/PULL_REQUEST_TEMPLATE.md", + ".forgejo/pull_request_template.md", + ".gitea/PULL_REQUEST_TEMPLATE.md", + ".gitea/pull_request_template.md", + ".github/PULL_REQUEST_TEMPLATE.md", + ".github/pull_request_template.md", + } + + createBaseRepo := func(t *testing.T, templateFiles []string, message string) (*repo_model.Repository, func()) { + t.Helper() + + changeOps := make([]*files_service.ChangeRepoFile, len(templateFiles)) + for i, template := range templateFiles { + changeOps[i] = &files_service.ChangeRepoFile{ + Operation: "create", + TreePath: template, + ContentReader: strings.NewReader(message + " " + template), + } + } + + repo, _, deferrer := CreateDeclarativeRepo(t, baseUser, "", nil, nil, changeOps) + + return repo, deferrer + } + + testPullPreview := func(t *testing.T, session *TestSession, user, repo, message string) { + t.Helper() + + req := NewRequest(t, "GET", path.Join(user, repo)) + resp := session.MakeRequest(t, req, http.StatusOK) + + // Click the PR button to create a pull + htmlDoc := NewHTMLParser(t, resp.Body) + link, exists := htmlDoc.doc.Find("#new-pull-request").Attr("href") + assert.True(t, exists, "The template has changed") + + // Load the pull request preview + req = NewRequest(t, "GET", link) + resp = session.MakeRequest(t, req, http.StatusOK) + + // Check that the message from the template is present. + htmlDoc = NewHTMLParser(t, resp.Body) + pullRequestMessage := htmlDoc.doc.Find("textarea[placeholder*='comment']").Text() + assert.Equal(t, message, pullRequestMessage) + } + + for i, template := range templateCandidates { + t.Run(template, func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Create the base repository, with the pull request template added. + message := fmt.Sprintf("TestPullCreateWithPullTemplate/%s", template) + baseRepo, deferrer := createBaseRepo(t, []string{template}, message) + defer deferrer() + + // Fork the repository + session := loginUser(t, forkUser.Name) + testRepoFork(t, session, baseUser.Name, baseRepo.Name, forkUser.Name, baseRepo.Name) + forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: forkUser.ID, Name: baseRepo.Name}) + + // Apply a change to the fork + err := createOrReplaceFileInBranch(forkUser, forkedRepo, "README.md", forkedRepo.DefaultBranch, fmt.Sprintf("Hello, World (%d)\n", i)) + assert.NoError(t, err) + + testPullPreview(t, session, forkUser.Name, forkedRepo.Name, message+" "+template) + }) + } + + t.Run("multiple template options", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Create the base repository, with the pull request template added. + message := "TestPullCreateWithPullTemplate/multiple" + baseRepo, deferrer := createBaseRepo(t, templateCandidates, message) + defer deferrer() + + // Fork the repository + session := loginUser(t, forkUser.Name) + testRepoFork(t, session, baseUser.Name, baseRepo.Name, forkUser.Name, baseRepo.Name) + forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: forkUser.ID, Name: baseRepo.Name}) + + // Apply a change to the fork + err := createOrReplaceFileInBranch(forkUser, forkedRepo, "README.md", forkedRepo.DefaultBranch, "Hello, World (%d)\n") + assert.NoError(t, err) + + // Unlike issues, where all candidates are considered and shown, for + // pull request, there's a priority: if there are multiple + // templates, only the highest priority one is used. + testPullPreview(t, session, forkUser.Name, forkedRepo.Name, message+" .forgejo/PULL_REQUEST_TEMPLATE.md") + }) + }) +} + func TestPullCreate_TitleEscape(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1")