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

Implement Issue Config #20956

Merged
merged 36 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
fac2cb6
Allow disabling blank Issues
JakobDev Aug 25, 2022
adb916e
Change supported paths
JakobDev Aug 29, 2022
c7a8ee4
Change config paths
JakobDev Aug 29, 2022
4d008fc
Merge branch 'main' into noblankissues
JakobDev Sep 2, 2022
ecbbbe9
Fix bugs
JakobDev Sep 2, 2022
7b4415b
Return error when getting default branch fails
JakobDev Sep 4, 2022
bdd3c2a
Merge branch 'main' into noblankissues
JakobDev Dec 13, 2022
146bd6b
Add documentation
JakobDev Dec 13, 2022
bb72aea
Refactor code
JakobDev Dec 20, 2022
8624c96
Improve documentation
JakobDev Dec 26, 2022
714e92e
Add API
JakobDev Jan 2, 2023
18fe929
Make error text translatable
JakobDev Jan 2, 2023
099def7
Apply suggestion from wolfogre
JakobDev Jan 9, 2023
fe94b82
Merge branch 'main' into noblankissues
JakobDev Jan 23, 2023
5ae2ed0
Fix linting
JakobDev Jan 23, 2023
0564f5c
Run make generate-swagger
JakobDev Jan 23, 2023
b34913f
Add defer reader.Close()
JakobDev Jan 27, 2023
c1dc3e4
Merge branch 'main' into noblankissues
JakobDev Feb 6, 2023
abf14a5
Implement Contact Links
JakobDev Feb 6, 2023
2a16447
Show choose page when contact links exists
JakobDev Feb 6, 2023
579b8e5
Validate ContactLinks
JakobDev Feb 7, 2023
1f52bb7
Add API to validate issue config
JakobDev Feb 7, 2023
ed09834
Fix lint
JakobDev Feb 7, 2023
0e01deb
Add API tests
JakobDev Feb 7, 2023
b054f49
Merge branch 'main' into noblankissues
JakobDev Feb 7, 2023
17185ab
Changes requested by wolfogre
JakobDev Feb 8, 2023
062375f
Run make fmt
JakobDev Feb 8, 2023
c820b91
Merge branch 'main' into noblankissues
JakobDev Feb 8, 2023
2ad3111
Merge branch 'main' into noblankissues
JakobDev Feb 13, 2023
b9750dc
Merge branch 'main' into noblankissues
JakobDev Feb 14, 2023
7a4c6f8
Merge branch 'main' into noblankissues
JakobDev Mar 27, 2023
159c0f7
Rename IssueConfigValidate to IssueConfigValidation
JakobDev Mar 27, 2023
68f508b
Missed test
JakobDev Mar 27, 2023
609840c
Typo
JakobDev Mar 27, 2023
31d1353
Merge branch 'main' into noblankissues
JakobDev Mar 28, 2023
d4a69d5
Merge branch 'main' into noblankissues
jolheiser Mar 28, 2023
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
67 changes: 67 additions & 0 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"fmt"
"html"
"io"
"net/http"
"net/url"
"path"
Expand All @@ -33,6 +34,7 @@ import (
asymkey_service "code.gitea.io/gitea/services/asymkey"

"github.com/editorconfig/editorconfig-core-go/v2"
"gopkg.in/yaml.v2"
JakobDev marked this conversation as resolved.
Show resolved Hide resolved
)

// IssueTemplateDirCandidates issue templates directory
Expand All @@ -47,6 +49,17 @@ var IssueTemplateDirCandidates = []string{
".gitlab/issue_template",
}

var IssueConfigCanidates = []string{
".gitea/ISSUE_TEMPLATE/config.yaml",
".gitea/ISSUE_TEMPLATE/config.yml",
".gitea/issue_template/config.yaml",
".gitea/issue_template/config.yml",
".github/ISSUE_TEMPLATE/config.yaml",
".github/ISSUE_TEMPLATE/config.yml",
".github/issue_template/config.yaml",
".github/issue_template/config.yml",
}
JakobDev marked this conversation as resolved.
Show resolved Hide resolved

// PullRequest contains information to make a pull request
type PullRequest struct {
BaseRepo *repo_model.Repository
Expand Down Expand Up @@ -1081,6 +1094,9 @@ func (ctx *Context) IssueTemplatesErrorsFromDefaultBranch() ([]*api.IssueTemplat
return issueTemplates, nil
}
for _, entry := range entries {
if entry.Name() == "config.yaml" || entry.Name() == "config.yml" {
JakobDev marked this conversation as resolved.
Show resolved Hide resolved
continue
}
if !template.CouldBe(entry.Name()) {
continue
}
Expand All @@ -1097,3 +1113,54 @@ func (ctx *Context) IssueTemplatesErrorsFromDefaultBranch() ([]*api.IssueTemplat
}
return issueTemplates, invalidFiles
}

func ExtractIssueConfigFromYaml(configContent []byte) (api.IssueConfig, error) {
JakobDev marked this conversation as resolved.
Show resolved Hide resolved
config := api.IssueConfig{
BlankIssuesEnabled: true,
JakobDev marked this conversation as resolved.
Show resolved Hide resolved
}

JakobDev marked this conversation as resolved.
Show resolved Hide resolved
err := yaml.Unmarshal(configContent, &config)
if err != nil {
return api.IssueConfig{}, err
}

return config, nil
}

func (ctx *Context) IssueConfigFromDefaultBranch() (api.IssueConfig, error) {
defaultIssueConfig := api.IssueConfig{
BlankIssuesEnabled: true,
}

commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch)
if err != nil {
JakobDev marked this conversation as resolved.
Show resolved Hide resolved
return defaultIssueConfig, wee
}

for _, configName := range IssueConfigCanidates {
entry, err := commit.GetTreeEntryByPath(configName)
if err != nil {
continue
}

r, err := entry.Blob().DataAsync()
if err != nil {
log.Debug("DataAsync: %v", err)
return defaultIssueConfig, nil
}

configContent, err := io.ReadAll(r)
if err != nil {
return defaultIssueConfig, err
}

issueConfig, err := ExtractIssueConfigFromYaml(configContent)
if err != nil {
return defaultIssueConfig, err
}

return issueConfig, nil
}

return defaultIssueConfig, nil
}
4 changes: 4 additions & 0 deletions modules/structs/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ func (l *IssueTemplateLabels) UnmarshalYAML(value *yaml.Node) error {
return fmt.Errorf("line %d: cannot unmarshal %s into IssueTemplateLabels", value.Line, value.ShortTag())
}

type IssueConfig struct {
BlankIssuesEnabled bool `json:"blank_issues_enabled" yaml:"blank_issues_enabled"`
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I've noticed from having to work with the GitHub templates already:
Collaborators who already have a lot of in-depth knowledge about a project tend to specify their issues with enough detail that they don't necessarily need to input all troubleshooting information.
Having to fill out the template can be pretty tiresome, especially for large projects like Gitea itself, and often leads to entering unnecessary information simply to satisfy the validation.
This means that later on, another value for-contributors, for example, could be added.
So, we could think about converting the type to (bool/)string, to support more use cases later on.
You don't need to implement the additional functionality I mentioned yet as that can be postponed.
However, I think converting it into a string can already be done in this PR.
As far as I know, it should not make a difference for the yaml parser.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that having such a setting would be useful, but @lunny requested compatibility with GitHub above, that's the reason it also searches in .github for the config. On GitHub, blank_issues_enabled is a boolean. That doesn't mean we can't add our own settings, but if we want compatibility with GitHub, we should not use a different type for the same setting as GitHub. Another setting would be the better solution here.

Copy link
Member

@delvh delvh Dec 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it would accept the same things as GitHub: true, yes, on, off, no, false (-> strconv.ParseBool),
and then more (i.e. for-contributors).
So it would still be compatible with GitHub?
As far as I know, you don't need " around a YAML string…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, you don't need " around a YAML string…

That's true in most cases, but reserved keywords like true or false or numbers needs quotes to be accepted as string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. Didn't know that.
But can our YAML Parser perhaps reconvert a boolean to a string?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw the parser seems to gracefully handle it, though I would need to test it in this context specifically.
https://go.dev/play/p/jNdWPWtK-Jz

Copy link
Member

@delvh delvh Dec 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's way better and easier to just add another setting like enable_blank_issues_for_contributors.

I strongly disagree there.
That simply allows for an ungodly amount of edge cases and illegal states.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw the parser seems to gracefully handle it, though I would need to test it in this context specifically. https://go.dev/play/p/jNdWPWtK-Jz

That's exactly what I hoped for.
This means we can still convert it without repercussions.
But I agree with @jolheiser, we should certainly ensure with a test that this behavior is fulfilled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is a bad idea. Even is that could parsed, this will confuse users and is still not comptaible with GitHub. I will keep the current behaviour for this PR. It is better to do is in another PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just keep it only allow true and false in this PR. And in future PR, it can be allowed other values but we don't need to discuss it in this PR.


// IssueTemplateType defines issue template type
type IssueTemplateType string

Expand Down
4 changes: 4 additions & 0 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,10 @@ func NewIssueChooseTemplate(ctx *context.Context) {
return
}

issueConfig, err := ctx.IssueConfigFromDefaultBranch()
ctx.Data["IssueConfig"] = issueConfig
ctx.Data["IssueConfigError"] = err
JakobDev marked this conversation as resolved.
Show resolved Hide resolved

ctx.Data["milestone"] = ctx.FormInt64("milestone")
ctx.Data["project"] = ctx.FormInt64("project")

Expand Down
26 changes: 18 additions & 8 deletions templates/repo/issue/choose.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,27 @@
</div>
</div>
{{end}}
<div class="ui attached segment">
<div class="ui two column grid">
<div class="column left aligned">
<strong>{{.locale.Tr "repo.issues.choose.blank"}}</strong>
<br/>{{.locale.Tr "repo.issues.choose.blank_about"}}
{{if .IssueConfig.BlankIssuesEnabled}}
<div class="ui attached segment">
<div class="ui two column grid">
<div class="column left aligned">
<strong>{{.locale.Tr "repo.issues.choose.blank"}}</strong>
<br/>{{.locale.Tr "repo.issues.choose.blank_about"}}
</div>
<div class="column right aligned">
<a href="{{.RepoLink}}/issues/new?{{if .milestone}}&milestone={{.milestone}}{{end}}{{if $.project}}&project={{$.project}}{{end}}" class="ui green button">{{$.locale.Tr "repo.issues.choose.get_started"}}</a>
</div>
</div>
<div class="column right aligned">
<a href="{{.RepoLink}}/issues/new?{{if .milestone}}&milestone={{.milestone}}{{end}}{{if $.project}}&project={{$.project}}{{end}}" class="ui green button">{{$.locale.Tr "repo.issues.choose.get_started"}}</a>
</div>
{{end}}
{{- if .IssueConfigError}}
JakobDev marked this conversation as resolved.
Show resolved Hide resolved
<div class="ui warning message">
<div class="text left">
<div>Your issue_config.yaml contains a error:</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that be a part of options/locale/locale_en-US.ini?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. I forgot to change it from my tests. I will do it later, when I have time.

<diy>{{.IssueConfigError}}</div>
</div>
</div>
</div>
{{end}}
JakobDev marked this conversation as resolved.
Show resolved Hide resolved
</div>
</div>
{{template "base/footer" .}}