From be8d16438a270211ffd6cfcce678b4d3b47d007a Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 5 Feb 2024 12:42:52 +0100 Subject: [PATCH] Fix `/api/v1/{owner}/{repo}/issue_templates` When issue templates were moved into services in def4956122ea2364f247712b13856383ee496add, the code was also refactored and simplified. Unfortunately, that simplification broke the `/api/v1/{owner}/{repo}/issue_templates` route, because it was previously using a helper function that ignored invalid templates, and after the refactor, the function it called *always* returned non-nil as the second return value. This, in turn, results in the aforementioned end point always returning an internal server error. This change restores the previous behaviour of ignoring invalid files returned by `issue.GetTemplatesFromDefaultBranch`, and adds a few test cases to exercise the endpoint. Other users of `GetTemplatesFromDefaultBranch` already ignore the second return value, or handle it correctly, so no changes are necessary there. Signed-off-by: Gergely Nagy --- routers/api/v1/repo/repo.go | 6 +- tests/integration/api_issue_templates_test.go | 64 +++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 tests/integration/api_issue_templates_test.go diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index b7ac5b4543..08e79e544a 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -1174,11 +1174,7 @@ 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.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo) ctx.JSON(http.StatusOK, ret) } diff --git a/tests/integration/api_issue_templates_test.go b/tests/integration/api_issue_templates_test.go new file mode 100644 index 0000000000..0d3ddb21bd --- /dev/null +++ b/tests/integration/api_issue_templates_test.go @@ -0,0 +1,64 @@ +// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "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" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +func TestAPIIssueTemplateList(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + t.Run("no templates", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + 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.Empty(t, issueTemplates) + }) + + t.Run("existing template", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + defer func() { + deleteFileInBranch(user, repo, "ISSUE_TEMPLATE/test.md", repo.DefaultBranch) + }() + + err := createOrReplaceFileInBranch(user, repo, "ISSUE_TEMPLATE/test.md", 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) + }) + }) +}