From 49919c636e4788a14f3762a0d9137ee2e0092790 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Mon, 13 Feb 2023 07:09:52 +0100 Subject: [PATCH] Pull Requests: setting to allow edits by maintainers by default, tweak UI (#22862) Add setting to allow edits by maintainers by default, to avoid having to often ask contributors to enable this. This also reorganizes the pull request settings UI to improve clarity. It was unclear which checkbox options were there to control available merge styles and which merge styles they correspond to. Now there is a "Merge Styles" label followed by the merge style options with the same name as in other menus. The remaining checkboxes were moved to the bottom, ordered rougly by typical order of operations. --------- Co-authored-by: Lunny Xiao --- models/repo/repo_unit.go | 1 + modules/structs/repo.go | 3 ++ options/locale/locale_en-US.ini | 9 ++-- routers/api/v1/repo/repo.go | 4 ++ routers/web/repo/compare.go | 7 +++ routers/web/repo/setting.go | 1 + services/convert/repository.go | 3 ++ services/forms/repo_form.go | 1 + templates/repo/issue/new_form.tmpl | 2 +- templates/repo/settings/options.tmpl | 66 ++++++++++++++++------------ templates/swagger/v1_json.tmpl | 9 ++++ 11 files changed, 72 insertions(+), 34 deletions(-) diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index ee450a46c4b..7c1af95bf09 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -125,6 +125,7 @@ type PullRequestsConfig struct { AllowRebaseUpdate bool DefaultDeleteBranchAfterMerge bool DefaultMergeStyle MergeStyle + DefaultAllowMaintainerEdit bool } // FromDB fills up a PullRequestsConfig from serialized format. diff --git a/modules/structs/repo.go b/modules/structs/repo.go index ee4bec4df79..b5a26a81559 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -96,6 +96,7 @@ type Repository struct { AllowRebaseUpdate bool `json:"allow_rebase_update"` DefaultDeleteBranchAfterMerge bool `json:"default_delete_branch_after_merge"` DefaultMergeStyle string `json:"default_merge_style"` + DefaultAllowMaintainerEdit bool `json:"default_allow_maintainer_edit"` AvatarURL string `json:"avatar_url"` Internal bool `json:"internal"` MirrorInterval string `json:"mirror_interval"` @@ -187,6 +188,8 @@ type EditRepoOption struct { DefaultDeleteBranchAfterMerge *bool `json:"default_delete_branch_after_merge,omitempty"` // set to a merge style to be used by this repository: "merge", "rebase", "rebase-merge", or "squash". DefaultMergeStyle *string `json:"default_merge_style,omitempty"` + // set to `true` to allow edits from maintainers by default + DefaultAllowMaintainerEdit *bool `json:"default_allow_maintainer_edit,omitempty"` // set to `true` to archive this repository. Archived *bool `json:"archived,omitempty"` // set to a string like `8h30m0s` to set the mirror interval time diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 5d0fd044f42..59089fd39bb 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1871,14 +1871,10 @@ settings.enable_timetracker = Enable Time Tracking settings.allow_only_contributors_to_track_time = Let Only Contributors Track Time settings.pulls_desc = Enable Repository Pull Requests settings.pulls.ignore_whitespace = Ignore Whitespace for Conflicts -settings.pulls.allow_merge_commits = Enable Commit Merging -settings.pulls.allow_rebase_merge = Enable Rebasing to Merge Commits -settings.pulls.allow_rebase_merge_commit = Enable Rebasing with explicit merge commits (--no-ff) -settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits -settings.pulls.allow_manual_merge = Enable Mark PR as manually merged settings.pulls.enable_autodetect_manual_merge = Enable autodetect manual merge (Note: In some special cases, misjudgments can occur) settings.pulls.allow_rebase_update = Enable updating pull request branch by rebase settings.pulls.default_delete_branch_after_merge = Delete pull request branch after merge by default +settings.pulls.default_allow_edits_from_maintainers = Allow edits from maintainers by default settings.releases_desc = Enable Repository Releases settings.packages_desc = Enable Repository Packages Registry settings.projects_desc = Enable Repository Projects @@ -2147,7 +2143,8 @@ settings.block_on_official_review_requests_desc = Merging will not be possible w settings.block_outdated_branch = Block merge if pull request is outdated settings.block_outdated_branch_desc = Merging will not be possible when head branch is behind base branch. settings.default_branch_desc = Select a default repository branch for pull requests and code commits: -settings.default_merge_style_desc = Default merge style for pull requests: +settings.merge_style_desc = Merge Styles +settings.default_merge_style_desc = Default Merge Style settings.choose_branch = Choose a branch… settings.no_protected_branch = There are no protected branches. settings.edit_protected_branch = Edit diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 1426d1dbccb..f5db45ffea0 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -863,6 +863,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { AllowRebaseUpdate: true, DefaultDeleteBranchAfterMerge: false, DefaultMergeStyle: repo_model.MergeStyleMerge, + DefaultAllowMaintainerEdit: false, } } else { config = unit.PullRequestsConfig() @@ -898,6 +899,9 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { if opts.DefaultMergeStyle != nil { config.DefaultMergeStyle = repo_model.MergeStyle(*opts.DefaultMergeStyle) } + if opts.DefaultAllowMaintainerEdit != nil { + config.DefaultAllowMaintainerEdit = *opts.DefaultAllowMaintainerEdit + } units = append(units, repo_model.RepoUnit{ RepoID: repo.ID, diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index c4b8c814e67..5b6faf4d83b 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -820,6 +820,13 @@ func CompareDiff(ctx *context.Context) { ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWrite(unit.TypePullRequests) + if unit, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests); err == nil { + config := unit.PullRequestsConfig() + ctx.Data["AllowMaintainerEdit"] = config.DefaultAllowMaintainerEdit + } else { + ctx.Data["AllowMaintainerEdit"] = false + } + ctx.HTML(http.StatusOK, tplCompare) } diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index 5c30795f228..436438c4366 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -529,6 +529,7 @@ func SettingsPost(ctx *context.Context) { AllowRebaseUpdate: form.PullsAllowRebaseUpdate, DefaultDeleteBranchAfterMerge: form.DefaultDeleteBranchAfterMerge, DefaultMergeStyle: repo_model.MergeStyle(form.PullsDefaultMergeStyle), + DefaultAllowMaintainerEdit: form.DefaultAllowMaintainerEdit, }, }) } else if !unit_model.TypePullRequests.UnitGlobalDisabled() { diff --git a/services/convert/repository.go b/services/convert/repository.go index ce53a666923..3ba604002ef 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -81,6 +81,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, mode perm.Acc allowRebaseUpdate := false defaultDeleteBranchAfterMerge := false defaultMergeStyle := repo_model.MergeStyleMerge + defaultAllowMaintainerEdit := false if unit, err := repo.GetUnit(ctx, unit_model.TypePullRequests); err == nil { config := unit.PullRequestsConfig() hasPullRequests = true @@ -92,6 +93,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, mode perm.Acc allowRebaseUpdate = config.AllowRebaseUpdate defaultDeleteBranchAfterMerge = config.DefaultDeleteBranchAfterMerge defaultMergeStyle = config.GetDefaultMergeStyle() + defaultAllowMaintainerEdit = config.DefaultAllowMaintainerEdit } hasProjects := false if _, err := repo.GetUnit(ctx, unit_model.TypeProjects); err == nil { @@ -182,6 +184,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, mode perm.Acc AllowRebaseUpdate: allowRebaseUpdate, DefaultDeleteBranchAfterMerge: defaultDeleteBranchAfterMerge, DefaultMergeStyle: string(defaultMergeStyle), + DefaultAllowMaintainerEdit: defaultAllowMaintainerEdit, AvatarURL: repo.AvatarLink(), Internal: !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePrivate, MirrorInterval: mirrorInterval, diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index db336e25e3d..c1b58009688 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -160,6 +160,7 @@ type RepoSettingForm struct { EnableAutodetectManualMerge bool PullsAllowRebaseUpdate bool DefaultDeleteBranchAfterMerge bool + DefaultAllowMaintainerEdit bool EnableTimetracker bool AllowOnlyContributorsToTrackTime bool EnableIssueDependencies bool diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index ed1fd4778f7..2e556867251 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -238,7 +238,7 @@
- +
{{end}} diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 6715a3c4eb0..7b8f9923afa 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -478,59 +478,41 @@
-
- - -
+

+ {{.locale.Tr "repo.settings.merge_style_desc"}} +

- +
- +
- +
- +
- -
-
-
-
- - -
-
-
-
- - -
-
-
-
- - +
+

{{.locale.Tr "repo.settings.default_merge_style_desc"}} @@ -564,6 +546,36 @@

+
+
+ + +
+
+
+
+ + +
+
+
+
+ + +
+
+
+
+ + +
+
+
+
+ + +
+
{{end}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 5b07965d2a6..e096faf3f3b 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -16497,6 +16497,11 @@ "type": "boolean", "x-go-name": "AutodetectManualMerge" }, + "default_allow_maintainer_edit": { + "description": "set to `true` to allow edits from maintainers by default", + "type": "boolean", + "x-go-name": "DefaultAllowMaintainerEdit" + }, "default_branch": { "description": "sets the default branch for this repository.", "type": "string", @@ -19015,6 +19020,10 @@ "format": "date-time", "x-go-name": "Created" }, + "default_allow_maintainer_edit": { + "type": "boolean", + "x-go-name": "DefaultAllowMaintainerEdit" + }, "default_branch": { "type": "string", "x-go-name": "DefaultBranch"