Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor issue template parsing and fix API endpoint (#29069) #29140

Merged
merged 2 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ func individualPermsChecker(ctx *context.APIContext) {
// check for and warn against deprecated authentication options
func checkDeprecatedAuthMethods(ctx *context.APIContext) {
if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" {
ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.")
ctx.Resp.Header().Set("X-Gitea-Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.")
}
}

Expand Down
10 changes: 5 additions & 5 deletions routers/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"slices"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -1159,12 +1160,11 @@ func GetIssueTemplates(ctx *context.APIContext) {
// "$ref": "#/responses/IssueTemplates"
// "404":
// "$ref": "#/responses/notFound"
ret, err := issue.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetTemplatesFromDefaultBranch", err)
return
ret := issue.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
if cnt := len(ret.TemplateErrors); cnt != 0 {
ctx.Resp.Header().Add("X-Gitea-Warning", "error occurs when parsing issue template: count="+strconv.Itoa(cnt))
}
ctx.JSON(http.StatusOK, ret)
ctx.JSON(http.StatusOK, ret.IssueTemplates)
}

// GetIssueConfig returns the issue config for a repo
Expand Down
20 changes: 9 additions & 11 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -963,19 +963,17 @@ func NewIssue(ctx *context.Context) {
}
ctx.Data["Tags"] = tags

_, templateErrs := issue_service.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ret := issue_service.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
templateLoaded, errs := setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates)
if len(errs) > 0 {
for k, v := range errs {
templateErrs[k] = v
}
for k, v := range errs {
ret.TemplateErrors[k] = v
}
if ctx.Written() {
return
}

if len(templateErrs) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, templateErrs), true)
if len(ret.TemplateErrors) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, ret.TemplateErrors), true)
}

ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWrite(unit.TypeIssues)
Expand Down Expand Up @@ -1018,11 +1016,11 @@ func NewIssueChooseTemplate(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.issues.new")
ctx.Data["PageIsIssueList"] = true

issueTemplates, errs := issue_service.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["IssueTemplates"] = issueTemplates
ret := issue_service.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["IssueTemplates"] = ret.IssueTemplates

if len(errs) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, errs), true)
if len(ret.TemplateErrors) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, ret.TemplateErrors), true)
}

if !issue_service.HasTemplatesOrContactLinks(ctx.Repo.Repository, ctx.Repo.GitRepo) {
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/milestone.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ func MilestoneIssuesAndPulls(ctx *context.Context) {

issues(ctx, milestoneID, projectID, util.OptionalBoolNone)

ret, _ := issue.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["NewIssueChooseTemplate"] = len(ret) > 0
ret := issue.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["NewIssueChooseTemplate"] = len(ret.IssueTemplates) > 0

ctx.Data["CanWriteIssues"] = ctx.Repo.CanWriteIssuesOrPulls(false)
ctx.Data["CanWritePulls"] = ctx.Repo.CanWriteIssuesOrPulls(true)
Expand Down
30 changes: 16 additions & 14 deletions services/issue/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,21 +109,23 @@ func IsTemplateConfig(path string) bool {
return false
}

// GetTemplatesFromDefaultBranch checks for issue templates in the repo's default branch,
// returns valid templates and the errors of invalid template files.
func GetTemplatesFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repository) ([]*api.IssueTemplate, map[string]error) {
var issueTemplates []*api.IssueTemplate

// ParseTemplatesFromDefaultBranch parses the issue templates in the repo's default branch,
// returns valid templates and the errors of invalid template files (the errors map is guaranteed to be non-nil).
func ParseTemplatesFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repository) (ret struct {
IssueTemplates []*api.IssueTemplate
TemplateErrors map[string]error
},
) {
ret.TemplateErrors = map[string]error{}
if repo.IsEmpty {
return issueTemplates, nil
return ret
}

commit, err := gitRepo.GetBranchCommit(repo.DefaultBranch)
if err != nil {
return issueTemplates, nil
return ret
}

invalidFiles := map[string]error{}
for _, dirName := range templateDirCandidates {
tree, err := commit.SubTree(dirName)
if err != nil {
Expand All @@ -133,24 +135,24 @@ func GetTemplatesFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repositor
entries, err := tree.ListEntries()
if err != nil {
log.Debug("list entries in %s: %v", dirName, err)
return issueTemplates, nil
return ret
}
for _, entry := range entries {
if !template.CouldBe(entry.Name()) {
continue
}
fullName := path.Join(dirName, entry.Name())
if it, err := template.UnmarshalFromEntry(entry, dirName); err != nil {
invalidFiles[fullName] = err
ret.TemplateErrors[fullName] = err
} else {
if !strings.HasPrefix(it.Ref, "refs/") { // Assume that the ref intended is always a branch - for tags users should use refs/tags/<ref>
it.Ref = git.BranchPrefix + it.Ref
}
issueTemplates = append(issueTemplates, it)
ret.IssueTemplates = append(ret.IssueTemplates, it)
}
}
}
return issueTemplates, invalidFiles
return ret
}

// GetTemplateConfigFromDefaultBranch returns the issue config for this repo.
Expand Down Expand Up @@ -179,8 +181,8 @@ func GetTemplateConfigFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repo
}

func HasTemplatesOrContactLinks(repo *repo.Repository, gitRepo *git.Repository) bool {
ret, _ := GetTemplatesFromDefaultBranch(repo, gitRepo)
if len(ret) > 0 {
ret := ParseTemplatesFromDefaultBranch(repo, gitRepo)
if len(ret.IssueTemplates) > 0 {
return true
}

Expand Down
55 changes: 55 additions & 0 deletions tests/integration/api_issue_templates_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package integration

import (
"net/http"
"net/url"
"testing"

repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
api "code.gitea.io/gitea/modules/structs"

"github.com/stretchr/testify/assert"
)

func TestAPIIssueTemplateList(t *testing.T) {
onGiteaRun(t, func(*testing.T, *url.URL) {
var issueTemplates []*api.IssueTemplate

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})

// no issue template
req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/issue_templates")
resp := MakeRequest(t, req, http.StatusOK)
issueTemplates = nil
DecodeJSON(t, resp, &issueTemplates)
assert.Empty(t, issueTemplates)

// one correct issue template and some incorrect issue templates
err := createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/tmpl-ok.md", repo.DefaultBranch, `----
name: foo
about: bar
----
`)
assert.NoError(t, err)

err = createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/tmpl-err1.yml", repo.DefaultBranch, `name: '`)
assert.NoError(t, err)

err = createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/tmpl-err2.yml", repo.DefaultBranch, `other: `)
assert.NoError(t, err)

req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/issue_templates")
resp = MakeRequest(t, req, http.StatusOK)
issueTemplates = nil
DecodeJSON(t, resp, &issueTemplates)
assert.Len(t, issueTemplates, 1)
assert.Equal(t, "foo", issueTemplates[0].Name)
assert.Equal(t, "error occurs when parsing issue template: count=2", resp.Header().Get("X-Gitea-Warning"))
})
}
Loading