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 <xiaolunwen@gmail.com>

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.

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

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

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,

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)
 }
 

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() {

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,

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

templates/repo/issue/new_form.tmpl

@@ -238,7 +238,7 @@
 				<div class="inline field">
 					<div class="ui checkbox">
 						<label class="tooltip" data-content="{{.locale.Tr "repo.pulls.allow_edits_from_maintainers_desc"}}"><strong>{{.locale.Tr "repo.pulls.allow_edits_from_maintainers"}}</strong></label>
-						<input name="allow_maintainer_edit" type="checkbox">
+						<input name="allow_maintainer_edit" type="checkbox" {{if .AllowMaintainerEdit}}checked{{end}}>
 					</div>
 				</div>
 			{{end}}

templates/repo/settings/options.tmpl

@@ -478,59 +478,41 @@
 					</div>
 					<div class="field{{if not $pullRequestEnabled}} disabled{{end}}" id="pull_box">
 						<div class="field">
-							<div class="ui checkbox">
+							<p>
-								<input name="pulls_ignore_whitespace" type="checkbox" {{if and $pullRequestEnabled ($prUnit.PullRequestsConfig.IgnoreWhitespaceConflicts)}}checked{{end}}>
+								{{.locale.Tr "repo.settings.merge_style_desc"}}
-								<label>{{.locale.Tr "repo.settings.pulls.ignore_whitespace"}}</label>
+							</p>
-							</div>
 						</div>
 						<div class="field">
 							<div class="ui checkbox">
 								<input name="pulls_allow_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowMerge)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.allow_merge_commits"}}</label>
+								<label>{{.locale.Tr "repo.pulls.merge_pull_request"}}</label>
 							</div>
 						</div>
 						<div class="field">
 							<div class="ui checkbox">
 								<input name="pulls_allow_rebase" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowRebase)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.allow_rebase_merge"}}</label>
+								<label>{{.locale.Tr "repo.pulls.rebase_merge_pull_request"}}</label>
 							</div>
 						</div>
 						<div class="field">
 							<div class="ui checkbox">
 								<input name="pulls_allow_rebase_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowRebaseMerge)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.allow_rebase_merge_commit"}}</label>
+								<label>{{.locale.Tr "repo.pulls.rebase_merge_commit_pull_request"}}</label>
 							</div>
 						</div>
 						<div class="field">
 							<div class="ui checkbox">
 								<input name="pulls_allow_squash" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowSquash)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.allow_squash_commits"}}</label>
+								<label>{{.locale.Tr "repo.pulls.squash_merge_pull_request"}}</label>
 							</div>
 						</div>
 						<div class="field">
 							<div class="ui checkbox">
 								<input name="pulls_allow_manual_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowManualMerge)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.allow_manual_merge"}}</label>
+								<label>{{.locale.Tr "repo.pulls.merge_manually"}}</label>
-							</div>
-						</div>
-						<div class="field">
-							<div class="ui checkbox">
-								<input name="enable_autodetect_manual_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AutodetectManualMerge)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.enable_autodetect_manual_merge"}}</label>
-							</div>
-						</div>
-						<div class="field">
-							<div class="ui checkbox">
-								<input name="pulls_allow_rebase_update" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowRebaseUpdate)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.allow_rebase_update"}}</label>
-							</div>
-						</div>
-						<div class="field">
-							<div class="ui checkbox">
-								<input name="default_delete_branch_after_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge)}}checked{{end}}>
-								<label>{{.locale.Tr "repo.settings.pulls.default_delete_branch_after_merge"}}</label>
 							</div>
 						</div>
+
 						<div class="field">
 							<p>
 								{{.locale.Tr "repo.settings.default_merge_style_desc"}}
@@ -564,6 +546,36 @@
 								</div>
 							</div>
 						</div>
+						<div class="field">
+							<div class="ui checkbox">
+								<input name="default_allow_maintainer_edit" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.DefaultAllowMaintainerEdit)}}checked{{end}}>
+								<label>{{.locale.Tr "repo.settings.pulls.default_allow_edits_from_maintainers"}}</label>
+							</div>
+						</div>
+						<div class="field">
+							<div class="ui checkbox">
+								<input name="pulls_allow_rebase_update" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowRebaseUpdate)}}checked{{end}}>
+								<label>{{.locale.Tr "repo.settings.pulls.allow_rebase_update"}}</label>
+							</div>
+						</div>
+						<div class="field">
+							<div class="ui checkbox">
+								<input name="default_delete_branch_after_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge)}}checked{{end}}>
+								<label>{{.locale.Tr "repo.settings.pulls.default_delete_branch_after_merge"}}</label>
+							</div>
+						</div>
+						<div class="field">
+							<div class="ui checkbox">
+								<input name="enable_autodetect_manual_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AutodetectManualMerge)}}checked{{end}}>
+								<label>{{.locale.Tr "repo.settings.pulls.enable_autodetect_manual_merge"}}</label>
+							</div>
+						</div>
+						<div class="field">
+							<div class="ui checkbox">
+								<input name="pulls_ignore_whitespace" type="checkbox" {{if and $pullRequestEnabled ($prUnit.PullRequestsConfig.IgnoreWhitespaceConflicts)}}checked{{end}}>
+								<label>{{.locale.Tr "repo.settings.pulls.ignore_whitespace"}}</label>
+							</div>
+						</div>
 					</div>
 				{{end}}
 

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"