From 9d8178b3ac5f86a64f67ab7b5dea99347a5afe72 Mon Sep 17 00:00:00 2001 From: Lanre Adelowo Date: Sun, 10 Feb 2019 20:27:19 +0100 Subject: [PATCH] Add option to close issues via commit on a non master branch (#5992) * fixes #5957 * add tests to make sure config option is respected * use already defined struct * - use migration to make the flag repo wide not for the entire gitea instance Also note that the config value can still be set so as to be able to control the value for new repositories that are to be created - fix copy/paste error in copyright header year and rearrange import - use repo config instead of server config value to determine if a commit should close an issue - update testsuite * use global config only when creating a new repository * allow repo admin toggle feature via UI * fix typo and improve testcase * fix fixtures * add DEFAULT prefix to config value * fix test --- custom/conf/app.ini.sample | 2 + .../doc/advanced/config-cheat-sheet.en-us.md | 1 + models/action.go | 3 +- models/action_test.go | 34 +++++++++++++++ models/fixtures/issue.yml | 13 ++++++ models/fixtures/repository.yml | 3 +- models/issue_test.go | 2 +- models/migrations/migrations.go | 2 + models/migrations/v79.go | 27 ++++++++++++ models/repo.go | 30 ++++++------- modules/auth/repo_form.go | 5 ++- modules/setting/setting.go | 42 ++++++++++--------- options/locale/locale_en-US.ini | 1 + routers/repo/setting.go | 16 ++++--- templates/repo/settings/options.tmpl | 4 ++ 15 files changed, 141 insertions(+), 44 deletions(-) create mode 100644 models/migrations/v79.go diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index 5ef4fa05b..2f4386dc7 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -36,6 +36,8 @@ DISABLE_HTTP_GIT = false ACCESS_CONTROL_ALLOW_ORIGIN = ; Force ssh:// clone url instead of scp-style uri when default SSH port is used USE_COMPAT_SSH_URI = false +; Close issues as long as a commit on any branch marks it as fixed +DEFAULT_CLOSE_ISSUES_VIA_COMMITS_IN_ANY_BRANCH = false [repository.editor] ; List of file extensions for which lines should be wrapped in the CodeMirror editor diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index b01f600b7..aa3a491d2 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -65,6 +65,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `ACCESS_CONTROL_ALLOW_ORIGIN`: **\**: Value for Access-Control-Allow-Origin header, default is not to present. **WARNING**: This maybe harmful to you website if you do not give it a right value. +- `DEFAULT_CLOSE_ISSUES_VIA_COMMITS_IN_ANY_BRANCH`: **false**: Close an issue if a commit on a non default branch marks it as closed. ### Repository - Pull Request (`repository.pull-request`) - `WORK_IN_PROGRESS_PREFIXES`: **WIP:,\[WIP\]**: List of prefixes used in Pull Request diff --git a/models/action.go b/models/action.go index c886408b2..ca6bfaf66 100644 --- a/models/action.go +++ b/models/action.go @@ -539,7 +539,8 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra } // Change issue status only if the commit has been pushed to the default branch. - if repo.DefaultBranch != branchName { + // and if the repo is configured to allow only that + if repo.DefaultBranch != branchName && !repo.CloseIssuesViaCommitInAnyBranch { continue } diff --git a/models/action_test.go b/models/action_test.go index 0310b0ad5..96d6ddb6d 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -260,6 +260,40 @@ func TestUpdateIssuesCommit(t *testing.T) { CheckConsistencyFor(t, &Action{}) } +func TestUpdateIssuesCommit_Issue5957(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + + // Test that push to a non-default branch closes an issue. + pushCommits := []*PushCommit{ + { + Sha1: "abcdef1", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user4@example.com", + AuthorName: "User Four", + Message: "close #2", + }, + } + + repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository) + commentBean := &Comment{ + Type: CommentTypeCommitRef, + CommitSHA: "abcdef1", + PosterID: user.ID, + IssueID: 7, + } + + issueBean := &Issue{RepoID: repo.ID, Index: 2, ID: 7} + + AssertNotExistsBean(t, commentBean) + AssertNotExistsBean(t, issueBean, "is_closed=1") + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, "non-existing-branch")) + AssertExistsAndLoadBean(t, commentBean) + AssertExistsAndLoadBean(t, issueBean, "is_closed=1") + CheckConsistencyFor(t, &Action{}) +} + func testCorrectRepoAction(t *testing.T, opts CommitRepoActionOptions, actionBean *Action) { AssertNotExistsBean(t, actionBean) assert.NoError(t, CommitRepoAction(opts)) diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index 4de8c4fa7..01bd8b86f 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -73,3 +73,16 @@ num_comments: 0 created_unix: 946684850 updated_unix: 978307200 + +- + id: 7 + repo_id: 2 + index: 2 + poster_id: 2 + name: issue7 + content: content for the seventh issue + is_closed: false + is_pull: false + created_unix: 946684830 + updated_unix: 978307200 + diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index d412e52d8..f41576165 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -17,11 +17,12 @@ lower_name: repo2 name: repo2 is_private: true - num_issues: 1 + num_issues: 2 num_closed_issues: 1 num_pulls: 0 num_closed_pulls: 0 num_stars: 1 + close_issues_via_commit_in_any_branch: true - id: 3 diff --git a/models/issue_test.go b/models/issue_test.go index 3bda3304c..cec7e8b47 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -275,7 +275,7 @@ func TestGetUserIssueStats(t *testing.T) { YourRepositoriesCount: 2, AssignCount: 0, CreateCount: 2, - OpenCount: 1, + OpenCount: 2, ClosedCount: 2, }, }, diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 533ff9073..174e7b515 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -210,6 +210,8 @@ var migrations = []Migration{ NewMigration("add theme to users", addUserDefaultTheme), // v78 -> v79 NewMigration("rename repo is_bare to repo is_empty", renameRepoIsBareToIsEmpty), + // v79 -> v80 + NewMigration("add can close issues via commit in any branch", addCanCloseIssuesViaCommitInAnyBranch), } // Migrate database to current version diff --git a/models/migrations/v79.go b/models/migrations/v79.go new file mode 100644 index 000000000..e24639395 --- /dev/null +++ b/models/migrations/v79.go @@ -0,0 +1,27 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "code.gitea.io/gitea/modules/setting" + + "github.com/go-xorm/xorm" +) + +func addCanCloseIssuesViaCommitInAnyBranch(x *xorm.Engine) error { + + type Repository struct { + ID int64 `xorm:"pk autoincr"` + CloseIssuesViaCommitInAnyBranch bool `xorm:"NOT NULL DEFAULT false"` + } + + if err := x.Sync2(new(Repository)); err != nil { + return err + } + + _, err := x.Exec("UPDATE repository SET close_issues_via_commit_in_any_branch = ?", + setting.Repository.DefaultCloseIssuesViaCommitsInAnyBranch) + return err +} diff --git a/models/repo.go b/models/repo.go index c0e69b0e3..848a76fe8 100644 --- a/models/repo.go +++ b/models/repo.go @@ -197,13 +197,14 @@ type Repository struct { ExternalMetas map[string]string `xorm:"-"` Units []*RepoUnit `xorm:"-"` - IsFork bool `xorm:"INDEX NOT NULL DEFAULT false"` - ForkID int64 `xorm:"INDEX"` - BaseRepo *Repository `xorm:"-"` - Size int64 `xorm:"NOT NULL DEFAULT 0"` - IndexerStatus *RepoIndexerStatus `xorm:"-"` - IsFsckEnabled bool `xorm:"NOT NULL DEFAULT true"` - Topics []string `xorm:"TEXT JSON"` + IsFork bool `xorm:"INDEX NOT NULL DEFAULT false"` + ForkID int64 `xorm:"INDEX"` + BaseRepo *Repository `xorm:"-"` + Size int64 `xorm:"NOT NULL DEFAULT 0"` + IndexerStatus *RepoIndexerStatus `xorm:"-"` + IsFsckEnabled bool `xorm:"NOT NULL DEFAULT true"` + CloseIssuesViaCommitInAnyBranch bool `xorm:"NOT NULL DEFAULT false"` + Topics []string `xorm:"TEXT JSON"` CreatedUnix util.TimeStamp `xorm:"INDEX created"` UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` @@ -1373,13 +1374,14 @@ func CreateRepository(doer, u *User, opts CreateRepoOptions) (_ *Repository, err } repo := &Repository{ - OwnerID: u.ID, - Owner: u, - Name: opts.Name, - LowerName: strings.ToLower(opts.Name), - Description: opts.Description, - IsPrivate: opts.IsPrivate, - IsFsckEnabled: !opts.IsMirror, + OwnerID: u.ID, + Owner: u, + Name: opts.Name, + LowerName: strings.ToLower(opts.Name), + Description: opts.Description, + IsPrivate: opts.IsPrivate, + IsFsckEnabled: !opts.IsMirror, + CloseIssuesViaCommitInAnyBranch: setting.Repository.DefaultCloseIssuesViaCommitsInAnyBranch, } sess := x.NewSession() diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 58be0ec3a..1a67f2b88 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -14,7 +14,7 @@ import ( "github.com/Unknwon/com" "github.com/go-macaron/binding" - "gopkg.in/macaron.v1" + macaron "gopkg.in/macaron.v1" ) // _______________________________________ _________.______________________ _______________.___. @@ -120,7 +120,8 @@ type RepoSettingForm struct { IsArchived bool // Admin settings - EnableHealthCheck bool + EnableHealthCheck bool + EnableCloseIssuesViaCommitInAnyBranch bool } // Validate validates the fields diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 0e3dc4688..d3b45ec29 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -190,16 +190,17 @@ var ( // Repository settings Repository = struct { - AnsiCharset string - ForcePrivate bool - DefaultPrivate string - MaxCreationLimit int - MirrorQueueLength int - PullRequestQueueLength int - PreferredLicenses []string - DisableHTTPGit bool - AccessControlAllowOrigin string - UseCompatSSHURI bool + AnsiCharset string + ForcePrivate bool + DefaultPrivate string + MaxCreationLimit int + MirrorQueueLength int + PullRequestQueueLength int + PreferredLicenses []string + DisableHTTPGit bool + AccessControlAllowOrigin string + UseCompatSSHURI bool + DefaultCloseIssuesViaCommitsInAnyBranch bool // Repository editor settings Editor struct { @@ -227,16 +228,17 @@ var ( WorkInProgressPrefixes []string } `ini:"repository.pull-request"` }{ - AnsiCharset: "", - ForcePrivate: false, - DefaultPrivate: RepoCreatingLastUserVisibility, - MaxCreationLimit: -1, - MirrorQueueLength: 1000, - PullRequestQueueLength: 1000, - PreferredLicenses: []string{"Apache License 2.0,MIT License"}, - DisableHTTPGit: false, - AccessControlAllowOrigin: "", - UseCompatSSHURI: false, + AnsiCharset: "", + ForcePrivate: false, + DefaultPrivate: RepoCreatingLastUserVisibility, + MaxCreationLimit: -1, + MirrorQueueLength: 1000, + PullRequestQueueLength: 1000, + PreferredLicenses: []string{"Apache License 2.0,MIT License"}, + DisableHTTPGit: false, + AccessControlAllowOrigin: "", + UseCompatSSHURI: false, + DefaultCloseIssuesViaCommitsInAnyBranch: false, // Repository editor settings Editor: struct { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 5001247e6..e0d5b64cf 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1035,6 +1035,7 @@ settings.pulls.allow_rebase_merge_commit = Enable Rebasing with explicit merge c settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits settings.admin_settings = Administrator Settings settings.admin_enable_health_check = Enable Repository Health Checks (git fsck) +settings.admin_enable_close_issues_via_commit_in_any_branch = Close an issue via a commit made in a non default branch settings.danger_zone = Danger Zone settings.new_owner_has_same_repo = The new owner already has a repository with same name. Please choose another name. settings.convert = Convert to Regular Repository diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 4fb74f6cf..5b5eaeb28 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -250,13 +250,19 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { if repo.IsFsckEnabled != form.EnableHealthCheck { repo.IsFsckEnabled = form.EnableHealthCheck - if err := models.UpdateRepository(repo, false); err != nil { - ctx.ServerError("UpdateRepository", err) - return - } - log.Trace("Repository admin settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) } + if repo.CloseIssuesViaCommitInAnyBranch != form.EnableCloseIssuesViaCommitInAnyBranch { + repo.CloseIssuesViaCommitInAnyBranch = form.EnableCloseIssuesViaCommitInAnyBranch + } + + if err := models.UpdateRepository(repo, false); err != nil { + ctx.ServerError("UpdateRepository", err) + return + } + + log.Trace("Repository admin settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) + ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success")) ctx.Redirect(ctx.Repo.RepoLink + "/settings") diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index e5a3ce075..432f20e74 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -263,6 +263,10 @@ +
+ + +