From 9903b8d7ec84b35e05d7056fcb4590c7069a54dc Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 28 Sep 2023 20:55:45 +0800 Subject: [PATCH] Fix Bug in Issue Config when only contact links are set (#26521) (#27334) Backport #26521 by @JakobDev Blank Issues should be enabled if they are not explicit disabled through the `blank_issues_enabled` field of the Issue Config. The Implementation has currently a Bug: If you create a Issue Config file with only `contact_links` and without a `blank_issues_enabled` field, `blank_issues_enabled` is set to false by default. The fix is only one line, but I decided to also improve the tests to make sure there are no other problems with the Implementation. This is a bugfix, so it should be backported to 1.20. Co-authored-by: JakobDev --- services/issue/template.go | 2 +- tests/integration/api_issue_config_test.go | 159 +++++++++++++++++++-- tests/integration/api_repo_file_helpers.go | 27 ++++ 3 files changed, 172 insertions(+), 16 deletions(-) diff --git a/services/issue/template.go b/services/issue/template.go index 4f1e3d93a..b6ae07798 100644 --- a/services/issue/template.go +++ b/services/issue/template.go @@ -72,7 +72,7 @@ func GetTemplateConfig(gitRepo *git.Repository, path string, commit *git.Commit) return GetDefaultTemplateConfig(), err } - issueConfig := api.IssueConfig{} + issueConfig := GetDefaultTemplateConfig() if err := yaml.Unmarshal(configContent, &issueConfig); err != nil { return GetDefaultTemplateConfig(), err } diff --git a/tests/integration/api_issue_config_test.go b/tests/integration/api_issue_config_test.go index f3dc663f3..b9125438b 100644 --- a/tests/integration/api_issue_config_test.go +++ b/tests/integration/api_issue_config_test.go @@ -15,38 +15,167 @@ import ( "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" ) -func TestAPIReposGetDefaultIssueConfig(t *testing.T) { - defer tests.PrepareTestEnv(t)() +func createIssueConfig(t *testing.T, user *user_model.User, repo *repo_model.Repository, issueConfig map[string]any) { + config, err := yaml.Marshal(issueConfig) + assert.NoError(t, err) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + err = createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/config.yaml", repo.DefaultBranch, string(config)) + assert.NoError(t, err) +} - urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issue_config", owner.Name, repo.Name) +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) resp := MakeRequest(t, req, http.StatusOK) var issueConfig api.IssueConfig DecodeJSON(t, resp, &issueConfig) - assert.True(t, issueConfig.BlankIssuesEnabled) - assert.Equal(t, issueConfig.ContactLinks, make([]api.IssueConfigContactLink, 0)) + return issueConfig } -func TestAPIReposValidateDefaultIssueConfig(t *testing.T) { +func TestAPIRepoGetIssueConfig(t *testing.T) { defer tests.PrepareTestEnv(t)() - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 49}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + t.Run("Default", func(t *testing.T) { + issueConfig := getIssueConfig(t, owner.Name, repo.Name) + + assert.True(t, issueConfig.BlankIssuesEnabled) + assert.Len(t, issueConfig.ContactLinks, 0) + }) + + t.Run("DisableBlankIssues", func(t *testing.T) { + config := make(map[string]any) + config["blank_issues_enabled"] = false + + createIssueConfig(t, owner, repo, config) + + issueConfig := getIssueConfig(t, owner.Name, repo.Name) + + assert.False(t, issueConfig.BlankIssuesEnabled) + assert.Len(t, issueConfig.ContactLinks, 0) + }) + + t.Run("ContactLinks", func(t *testing.T) { + contactLink := make(map[string]string) + contactLink["name"] = "TestName" + contactLink["url"] = "https://example.com" + contactLink["about"] = "TestAbout" + + config := make(map[string]any) + config["contact_links"] = []map[string]string{contactLink} + + createIssueConfig(t, owner, repo, config) + + issueConfig := getIssueConfig(t, owner.Name, repo.Name) + + assert.True(t, issueConfig.BlankIssuesEnabled) + assert.Len(t, issueConfig.ContactLinks, 1) + + assert.Equal(t, "TestName", issueConfig.ContactLinks[0].Name) + assert.Equal(t, "https://example.com", issueConfig.ContactLinks[0].URL) + assert.Equal(t, "TestAbout", issueConfig.ContactLinks[0].About) + }) + + t.Run("Full", func(t *testing.T) { + contactLink := make(map[string]string) + contactLink["name"] = "TestName" + contactLink["url"] = "https://example.com" + contactLink["about"] = "TestAbout" + + config := make(map[string]any) + config["blank_issues_enabled"] = false + config["contact_links"] = []map[string]string{contactLink} + + createIssueConfig(t, owner, repo, config) + + issueConfig := getIssueConfig(t, owner.Name, repo.Name) + + assert.False(t, issueConfig.BlankIssuesEnabled) + assert.Len(t, issueConfig.ContactLinks, 1) + + assert.Equal(t, "TestName", issueConfig.ContactLinks[0].Name) + assert.Equal(t, "https://example.com", issueConfig.ContactLinks[0].URL) + assert.Equal(t, "TestAbout", issueConfig.ContactLinks[0].About) + }) +} + +func TestAPIRepoIssueConfigPaths(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 49}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + templateConfigCandidates := []string{ + ".gitea/ISSUE_TEMPLATE/config", + ".gitea/issue_template/config", + ".github/ISSUE_TEMPLATE/config", + ".github/issue_template/config", + } + + for _, canidate := range templateConfigCandidates { + for _, extension := range []string{".yaml", ".yml"} { + fullPath := canidate + extension + t.Run(fullPath, func(t *testing.T) { + configMap := make(map[string]any) + configMap["blank_issues_enabled"] = false + + configData, err := yaml.Marshal(configMap) + assert.NoError(t, err) + + _, err = createFileInBranch(owner, repo, fullPath, repo.DefaultBranch, string(configData)) + assert.NoError(t, err) + + issueConfig := getIssueConfig(t, owner.Name, repo.Name) + + assert.False(t, issueConfig.BlankIssuesEnabled) + assert.Len(t, issueConfig.ContactLinks, 0) + + _, err = deleteFileInBranch(owner, repo, fullPath, repo.DefaultBranch) + assert.NoError(t, err) + }) + } + } +} + +func TestAPIRepoValidateIssueConfig(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 49}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issue_config/validate", owner.Name, repo.Name) - req := NewRequest(t, "GET", urlStr) - resp := MakeRequest(t, req, http.StatusOK) - var issueConfigValidation api.IssueConfigValidation - DecodeJSON(t, resp, &issueConfigValidation) + t.Run("Valid", func(t *testing.T) { + req := NewRequest(t, "GET", urlStr) + resp := MakeRequest(t, req, http.StatusOK) - assert.True(t, issueConfigValidation.Valid) - assert.Empty(t, issueConfigValidation.Message) + var issueConfigValidation api.IssueConfigValidation + DecodeJSON(t, resp, &issueConfigValidation) + + assert.True(t, issueConfigValidation.Valid) + assert.Empty(t, issueConfigValidation.Message) + }) + + t.Run("Invalid", func(t *testing.T) { + config := make(map[string]any) + config["blank_issues_enabled"] = "Test" + + createIssueConfig(t, owner, repo, config) + + req := NewRequest(t, "GET", urlStr) + resp := MakeRequest(t, req, http.StatusOK) + + var issueConfigValidation api.IssueConfigValidation + DecodeJSON(t, resp, &issueConfigValidation) + + assert.False(t, issueConfigValidation.Valid) + assert.NotEmpty(t, issueConfigValidation.Message) + }) } diff --git a/tests/integration/api_repo_file_helpers.go b/tests/integration/api_repo_file_helpers.go index 27e127fcc..4350092b8 100644 --- a/tests/integration/api_repo_file_helpers.go +++ b/tests/integration/api_repo_file_helpers.go @@ -6,6 +6,7 @@ package integration import ( "strings" + "code.gitea.io/gitea/models" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" @@ -29,6 +30,32 @@ func createFileInBranch(user *user_model.User, repo *repo_model.Repository, tree return files_service.ChangeRepoFiles(git.DefaultContext, repo, user, opts) } +func deleteFileInBranch(user *user_model.User, repo *repo_model.Repository, treePath, branchName string) (*api.FilesResponse, error) { + opts := &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "delete", + TreePath: treePath, + }, + }, + OldBranch: branchName, + Author: nil, + Committer: nil, + } + return files_service.ChangeRepoFiles(git.DefaultContext, repo, user, opts) +} + +func createOrReplaceFileInBranch(user *user_model.User, repo *repo_model.Repository, treePath, branchName, content string) error { + _, err := deleteFileInBranch(user, repo, treePath, branchName) + + if err != nil && !models.IsErrRepoFileDoesNotExist(err) { + return err + } + + _, err = createFileInBranch(user, repo, treePath, branchName, content) + return err +} + func createFile(user *user_model.User, repo *repo_model.Repository, treePath string) (*api.FilesResponse, error) { return createFileInBranch(user, repo, treePath, repo.DefaultBranch, "This is a NEW file") }