diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 55049de50c..bff4094f89 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -610,10 +610,9 @@ reviews should not grant this category. This category controls which users are allowed to upload new patch sets to existing changes. Irrespective of this permission, change owners are always -allowed to upload new patch sets for their changes. This permission needs to be -set on `refs/for/*`. +allowed to upload new patch sets for their changes. -By default, this permission is granted to `Registered Users` on `refs/for/*`, +By default, this permission is granted to `Registered Users` on `refs/*`, allowing all registered users to upload a new patch set to any change. Revoking this permission (by granting it to no groups and setting the "Exclusive" flag) will prevent users from uploading a patch set to a change they do not own. @@ -629,13 +628,6 @@ push to happen. Some projects wish to restrict merges to being created by Gerrit. By granting `Push` without `Push Merge Commit`, the only merges that enter the system will be those created by Gerrit. -The reference name connected to a `Push Merge Commit` entry must always -be prefixed with `refs/for/`, for example `refs/for/refs/heads/BRANCH`. -This applies even for an entry that complements a `Push` entry for -`refs/heads/BRANCH` that allows direct pushes of non-merge commits, and -the intention of the `Push Merge Commit` entry is to allow direct pushes -of merge commits. - [[category_push_annotated]] [[category_create_annotated]] @@ -795,8 +787,7 @@ above) must enable submit, and also must not block it. See above for details on each label. To link:user-upload.html#auto_merge[immediately submit a change on push] -the caller needs to have the Submit permission on `refs/for/` -(e.g. on `refs/for/refs/heads/master`). +the caller needs to enable the "Submit on Push" option. Submitting to the `refs/meta/config` branch is only allowed to project owners. Any explicit submit permissions for non-project-owners on this @@ -917,7 +908,7 @@ Suggested access rights to grant: * xref:category_read[`Read`] on 'refs/heads/\*' and 'refs/tags/*' * xref:category_push[`Create Review`] to 'refs/heads/*' -* xref:category_push_merge[`Push merge commit`] to 'refs/for/refs/heads/*' +* xref:category_push_merge[`Push merge commit`] to 'refs/heads/*' * xref:category_forge_author[`Forge Author Identity`] to 'refs/heads/*' * link:config-labels.html#label_Code-Review[`Label: Code-Review`] with range '-2' to '+2' for 'refs/heads/*' * link:config-labels.html#label_Verified[`Label: Verified`] with range '-1' to '+1' for 'refs/heads/*' diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionEditor.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionEditor.ui.xml index 00c41dc09f..8d251624af 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionEditor.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionEditor.ui.xml @@ -58,7 +58,7 @@ limitations under the License. position: absolute; top: 0; right: 36px; - width: 7em; + width: 9em; font-size: 80%; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java index 16dd1679c3..f8f33d0fec 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java @@ -16,6 +16,7 @@ package com.google.gerrit.client.admin; import static com.google.gerrit.common.data.Permission.EDIT_TOPIC_NAME; import static com.google.gerrit.common.data.Permission.PUSH; +import static com.google.gerrit.common.data.Permission.SUBMIT; import com.google.gerrit.client.Dispatcher; import com.google.gerrit.client.Gerrit; @@ -139,15 +140,23 @@ public class PermissionRuleEditor extends Composite initWidget(uiBinder.createAndBindUi(this)); String name = permission.getName(); - boolean canForce = PUSH.equals(name); - if (canForce) { - String ref = section.getName(); - canForce = !ref.startsWith("refs/for/") && !ref.startsWith("^refs/for/"); - force.setText(PermissionRule.FORCE_PUSH); - } else { - canForce = EDIT_TOPIC_NAME.equals(name); - force.setText(PermissionRule.FORCE_EDIT); + boolean canForce = true; + switch (name) { + case SUBMIT: + force.setText(PermissionRule.FORCE_SUBMIT); + break; + case EDIT_TOPIC_NAME: + force.setText(PermissionRule.FORCE_EDIT); + break; + case PUSH: + force.setText(PermissionRule.FORCE_PUSH); + String ref = section.getName(); + canForce = !ref.startsWith("refs/for/") && !ref.startsWith("^refs/for/"); + break; + default: + canForce = false; } + force.setVisible(canForce); force.setEnabled(!readOnly); action.getElement().setPropertyBoolean("disabled", readOnly); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.ui.xml index 644fef4349..aa38d279f2 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.ui.xml @@ -55,7 +55,7 @@ limitations under the License. position: absolute; top: 0; right: 36px; - width: 7em; + width: 9em; font-size: 80%; } diff --git a/java/com/google/gerrit/common/data/PermissionRule.java b/java/com/google/gerrit/common/data/PermissionRule.java index c50af5c78a..e6673d83b1 100644 --- a/java/com/google/gerrit/common/data/PermissionRule.java +++ b/java/com/google/gerrit/common/data/PermissionRule.java @@ -15,6 +15,7 @@ package com.google.gerrit.common.data; public class PermissionRule implements Comparable { + public static final String FORCE_SUBMIT = "Submit on Push"; public static final String FORCE_PUSH = "Force Push"; public static final String FORCE_EDIT = "Force Edit"; diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java index 990c318f21..529586e09f 100644 --- a/java/com/google/gerrit/server/permissions/ChangeControl.java +++ b/java/com/google/gerrit/server/permissions/ChangeControl.java @@ -141,7 +141,7 @@ class ChangeControl { /** Can this user rebase this change? */ private boolean canRebase(ReviewDb db) throws OrmException { - return (isOwner() || refControl.canSubmit(isOwner()) || refControl.canRebase()) + return (isOwner() || refControl.canSubmit(isOwner(), false) || refControl.canRebase()) && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE) && !isPatchSetLocked(db); } @@ -374,7 +374,7 @@ class ChangeControl { case RESTORE: return canRestore(db()); case SUBMIT: - return refControl.canSubmit(isOwner()); + return refControl.canSubmit(isOwner(), false); case REMOVE_REVIEWER: case SUBMIT_AS: diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java index 6a581016e4..bf3a1336a5 100644 --- a/java/com/google/gerrit/server/permissions/RefControl.java +++ b/java/com/google/gerrit/server/permissions/RefControl.java @@ -29,7 +29,6 @@ import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.permissions.PermissionBackend.ForChange; import com.google.gerrit.server.permissions.PermissionBackend.ForRef; import com.google.gerrit.server.query.change.ChangeData; -import com.google.gerrit.server.util.MagicBranch; import com.google.gwtorm.server.OrmException; import com.google.inject.util.Providers; import java.util.Collection; @@ -97,9 +96,7 @@ class RefControl { /** @return true if this user can add a new patch set to this ref */ boolean canAddPatchSet() { - return projectControl - .controlForRef(MagicBranch.NEW_CHANGE + refName) - .canPerform(Permission.ADD_PATCH_SET); + return canPerform(Permission.ADD_PATCH_SET); } /** @return true if this user can rebase changes on this ref */ @@ -108,7 +105,7 @@ class RefControl { } /** @return true if this user can submit patch sets to this ref */ - boolean canSubmit(boolean isChangeOwner) { + boolean canSubmit(boolean isChangeOwner, boolean force) { if (RefNames.REFS_CONFIG.equals(refName)) { // Always allow project owners to submit configuration changes. // Submitting configuration changes modifies the access control @@ -117,7 +114,7 @@ class RefControl { // granting of powers beyond submitting to the configuration. return projectControl.isOwner(); } - return canPerform(Permission.SUBMIT, isChangeOwner, false); + return canPerform(Permission.SUBMIT, isChangeOwner, force); } /** @return true if this user can force edit topic names. */ @@ -153,7 +150,7 @@ class RefControl { /** @return true if this user can submit merge patch sets to this ref */ private boolean canUploadMerges() { - return projectControl.controlForRef("refs/for/" + refName).canPerform(Permission.PUSH_MERGE); + return canPerform(Permission.PUSH_MERGE); } /** @return true if the user can update the reference as a fast-forward. */ @@ -494,7 +491,7 @@ class RefControl { return canPerform(refPermissionName(perm)); case UPDATE_BY_SUBMIT: - return projectControl.controlForRef(MagicBranch.NEW_CHANGE + refName).canSubmit(true); + return canSubmit(true, true); case READ_PRIVATE_CHANGES: return canPerform(Permission.VIEW_PRIVATE_CHANGES); diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java index d2fd7cef7d..9c6a19e85b 100644 --- a/java/com/google/gerrit/server/project/ProjectConfig.java +++ b/java/com/google/gerrit/server/project/ProjectConfig.java @@ -312,6 +312,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. remove(section); } else if (section != null) { AccessSection a = accessSections.get(section.getName()); + if (a == null) { + System.out.println(a); + } a.remove(permission); if (a.getPermissions().isEmpty()) { remove(a); @@ -703,19 +706,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. groupsByName, perm, Permission.hasRange(convertedName)); - if (migrateRefsFor(as, perm)) { - if (isRefsForExclusively(refName)) { - // Since the ref only applies on refs/for/* and no other - // namespaces, we can remove the old permission. - remove(as, perm); - } - } } else { sectionsWithUnknownPermissions.add(as.getName()); } } } } + migrateRefsFor(); AccessSection capability = null; for (String varName : rc.getNames(CAPABILITY)) { @@ -729,6 +726,20 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. } } + private void migrateRefsFor() { + for (AccessSection as : ImmutableList.copyOf(accessSections.values())) { + for (Permission p : ImmutableList.copyOf(as.getPermissions())) { + if (migrateRefsFor(as, p)) { + if (isRefsForExclusively(as.getName())) { + // Since the ref only applies on refs/for/* and no other + // namespaces, we can remove the old permission. + remove(as, p); + } + } + } + } + } + private boolean isRefsForExclusively(String refName) { return refName.startsWith("refs/for/"); } @@ -756,6 +767,35 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. convertedPerm.addAll(perm.getRules()); return true; } + if (isRefsForExclusively(refName) && perm.getName().equals(Permission.SUBMIT)) { + // We only migrate "Submit" if is exclusively added to refs/for/. + // We do this because when the new "Submit on Push" force + // option is added, it is somewhat likely that people will start specify + // Submit on refs/* (something which previously had to be avoided if the + // user did not want to enable "Submit on push"). + // Also, before this change, the documentation mentioned + // explicitly that "refs/for/*" will enable "Submit on push". + AccessSection migratedAs = getAccessSection(unRefsFor(refName), true); + Permission migratedPerm = migratedAs.getPermission(Permission.SUBMIT, true); + migratedPerm.setExclusiveGroup(perm.getExclusiveGroup()); + for (PermissionRule rule : perm.getRules()) { + PermissionRule migratedRule = migratedPerm.getRule(rule.getGroup(), true); + migratedRule.setForce(true); + } + return true; + } + if (isRefsForExclusively(refName) && perm.getName().equals(Permission.ADD_PATCH_SET)) { + // No need to migrate ADD_PATCH_SET on refs/* + AccessSection migratedAs = getAccessSection(unRefsFor(refName), true); + migratedAs.addPermission(perm); + return true; + } + if (isRefsForExclusively(refName) && perm.getName().equals(Permission.PUSH_MERGE)) { + // No need to migrate PUSH_MERGE on refs/* + AccessSection migratedAs = getAccessSection(unRefsFor(refName), true); + migratedAs.addPermission(perm); + return true; + } return false; } diff --git a/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/java/com/google/gerrit/server/schema/AllProjectsCreator.java index fe4b2758a7..8c5cabd856 100644 --- a/java/com/google/gerrit/server/schema/AllProjectsCreator.java +++ b/java/com/google/gerrit/server/schema/AllProjectsCreator.java @@ -162,13 +162,11 @@ public class AllProjectsCreator { AccessSection heads = config.getAccessSection(AccessSection.HEADS, true); AccessSection tags = config.getAccessSection("refs/tags/*", true); AccessSection meta = config.getAccessSection(RefNames.REFS_CONFIG, true); - AccessSection refsFor = config.getAccessSection("refs/for/*", true); - AccessSection magic = config.getAccessSection("refs/for/" + AccessSection.ALL, true); grant(config, cap, GlobalCapability.ADMINISTRATE_SERVER, admin); grant(config, all, Permission.READ, admin, anonymous); grant(config, all, Permission.CREATE_REVIEW, registered); - grant(config, refsFor, Permission.ADD_PATCH_SET, registered); + grant(config, all, Permission.ADD_PATCH_SET, registered); if (batch != null) { Permission priority = cap.getPermission(GlobalCapability.PRIORITY, true); @@ -186,6 +184,7 @@ public class AllProjectsCreator { grant(config, heads, cr, -2, 2, admin, owners); grant(config, heads, Permission.CREATE, admin, owners); grant(config, heads, Permission.PUSH, admin, owners); + grant(config, all, Permission.PUSH_MERGE, registered); grant(config, heads, Permission.SUBMIT, admin, owners); grant(config, heads, Permission.FORGE_AUTHOR, registered); grant(config, heads, Permission.FORGE_COMMITTER, admin, owners); @@ -195,8 +194,6 @@ public class AllProjectsCreator { grant(config, tags, Permission.CREATE_TAG, admin, owners); grant(config, tags, Permission.CREATE_SIGNED_TAG, admin, owners); - grant(config, magic, Permission.PUSH_MERGE, registered); - meta.getPermission(Permission.READ, true).setExclusiveGroup(true); grant(config, meta, Permission.READ, admin, owners); grant(config, meta, cr, -2, 2, admin, owners); diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index 5f19d50002..4d22ada7ee 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -2727,7 +2727,7 @@ public class ChangeIT extends AbstractDaemonTest { TestRepository userTestRepo = cloneProject(p, user); // Block default permission - block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); + block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); // Create change as admin PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo); @@ -2771,7 +2771,7 @@ public class ChangeIT extends AbstractDaemonTest { TestRepository adminTestRepo = cloneProject(project, admin); // Block default permission - block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); + block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); // Create change as admin PushOneCommit push = pushFactory.create(db, admin.getIdent(), adminTestRepo); @@ -3407,7 +3407,7 @@ public class ChangeIT extends AbstractDaemonTest { Project.NameKey p = createProject("addPatchSetEdit"); TestRepository userTestRepo = cloneProject(p, user); // Block default permission - block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); + block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); // Create change as user PushOneCommit push = pushFactory.create(db, user.getIdent(), userTestRepo); PushOneCommit.Result r = push.to("refs/for/master"); diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 91a12788c6..bd3daa387c 100644 --- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -694,7 +694,7 @@ public class ChangeEditIT extends AbstractDaemonTest { TestRepository userTestRepo = cloneProject(p, user); // Block default permission - block(p, "refs/for/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); + block(p, "refs/*", Permission.ADD_PATCH_SET, REGISTERED_USERS); // Create change as user PushOneCommit push = pushFactory.create(db, user.getIdent(), userTestRepo); diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java index 7e671dfa13..4a9179e25e 100644 --- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java +++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java @@ -129,11 +129,11 @@ public class RefControlTest { } private void assertCanSubmit(String ref, ProjectControl u) { - assertThat(u.controlForRef(ref).canSubmit(false)).named("can submit " + ref).isTrue(); + assertThat(u.controlForRef(ref).canSubmit(false, false)).named("can submit " + ref).isTrue(); } private void assertCannotSubmit(String ref, ProjectControl u) { - assertThat(u.controlForRef(ref).canSubmit(false)).named("can submit " + ref).isFalse(); + assertThat(u.controlForRef(ref).canSubmit(false, false)).named("can submit " + ref).isFalse(); } private void assertCanUpload(ProjectControl u) { diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java index 63338c45a8..533bfe80c9 100644 --- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java +++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java @@ -614,6 +614,125 @@ public class ProjectConfigTest extends GerritBaseTests { .isEqualTo(Lists.newArrayList(new PermissionRule(developers))); } + @Test + public void readConfigSubmitRefsForStarIsMigrated() throws Exception { + RevCommit rev = + tr.commit() + .add("groups", group(developers)) + .add("project.config", "[access \"refs/for/*\"]\n" + " submit = group Developers\n") + .create(); + + ProjectConfig cfg = read(rev); + AccessSection as = cfg.getAccessSection("refs/for/*"); + assertThat(as).isNull(); + + as = cfg.getAccessSection("refs/*"); + assertThat(as.getPermission(Permission.SUBMIT, false)).isNotNull(); + PermissionRule rule = new PermissionRule(developers); + rule.setForce(true); + assertThat(as.getPermission(Permission.SUBMIT, false).getRules()) + .isEqualTo(Lists.newArrayList(rule)); + } + + @Test + public void readConfigSubmitRefsStarIsNotMigrated() throws Exception { + RevCommit rev = + tr.commit() + .add("groups", group(developers)) + .add("project.config", "[access \"refs/*\"]\n" + " submit = group Developers\n") + .create(); + + ProjectConfig cfg = read(rev); + AccessSection as = cfg.getAccessSection("refs/*"); + assertThat(as).isNotNull(); + PermissionRule rule = new PermissionRule(developers); + assertThat(as.getPermission(Permission.SUBMIT, false).getRules()) + .isEqualTo(Lists.newArrayList(rule)); + } + + @Test + public void readConfigSubmitRefsForStarIsMigratedIntoExistingPermission() throws Exception { + RevCommit rev = + tr.commit() + .add("groups", group(developers)) + .add( + "project.config", + "[access \"refs/*\"]\n" + + " submit = group Developers\n" + + "[access \"refs/for/*\"]\n" + + " submit = group Developers\n") + .create(); + + ProjectConfig cfg = read(rev); + AccessSection as = cfg.getAccessSection("refs/*"); + assertThat(as).isNotNull(); + PermissionRule rule = new PermissionRule(developers); + rule.setForce(true); + assertThat(as.getPermission(Permission.SUBMIT, false).getRules()) + .isEqualTo(Lists.newArrayList(rule)); + } + + @Test + public void readConfigAddPatchSetRefsForStarIsMigrated() throws Exception { + RevCommit rev = + tr.commit() + .add("groups", group(developers)) + .add( + "project.config", + "[access \"refs/for/*\"]\n" + " addPatchSet = group Developers\n") + .create(); + + ProjectConfig cfg = read(rev); + AccessSection as = cfg.getAccessSection("refs/for/*"); + assertThat(as).isNull(); + as = cfg.getAccessSection("refs/*"); + assertThat(as).isNotNull(); + PermissionRule rule = new PermissionRule(developers); + assertThat(as.getPermission(Permission.ADD_PATCH_SET, false).getRules()) + .isEqualTo(Lists.newArrayList(rule)); + } + + @Test + public void readConfigPushMergeRefsForStarIsMigrated() throws Exception { + RevCommit rev = + tr.commit() + .add("groups", group(developers)) + .add("project.config", "[access \"refs/for/*\"]\n" + " pushMerge = group Developers\n") + .create(); + + ProjectConfig cfg = read(rev); + AccessSection as = cfg.getAccessSection("refs/for/*"); + assertThat(as).isNull(); + as = cfg.getAccessSection("refs/*"); + assertThat(as).isNotNull(); + PermissionRule rule = new PermissionRule(developers); + assertThat(as.getPermission(Permission.PUSH_MERGE, false).getRules()) + .isEqualTo(Lists.newArrayList(rule)); + } + + @Test + public void readConfigMultiplePermissionsOnRefsForStarIsMigrated() throws Exception { + RevCommit rev = + tr.commit() + .add("groups", group(developers)) + .add( + "project.config", + "[access \"refs/for/*\"]\n" + + " pushMerge = group Developers\n" + + " addPatchSet = group Developers\n") + .create(); + + ProjectConfig cfg = read(rev); + AccessSection as = cfg.getAccessSection("refs/for/*"); + assertThat(as).isNull(); + as = cfg.getAccessSection("refs/*"); + assertThat(as).isNotNull(); + assertThat(as.getPermission(Permission.PUSH_MERGE, false).getRules()) + .isEqualTo(Lists.newArrayList(new PermissionRule(developers))); + assertThat(as.getPermission(Permission.ADD_PATCH_SET, false).getRules()) + .isEqualTo(Lists.newArrayList(new PermissionRule(developers))); + } + @Test public void readCommentLinkMatchButNoHtmlOrLink() throws Exception { RevCommit rev = diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js index ce018090a7..d4c5fd14ba 100644 --- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js +++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js @@ -56,6 +56,17 @@ }, ]; + const FORCE_SUBMIT_OPTIONS = [ + { + name: 'No Submit on Push', + value: false, + }, + { + name: 'Submit on Push', + value: true, + }, + ]; + Polymer({ is: 'gr-rule-editor', @@ -114,7 +125,8 @@ _computeForce(permission) { return this.permissionValues.push.id === permission || - this.permissionValues.editTopicName.id === permission; + this.permissionValues.editTopicName.id === permission || + this.permissionValues.submit.id === permission; }, _computeForceClass(permission) { @@ -156,6 +168,8 @@ return FORCE_PUSH_OPTIONS; } else if (permission === this.permissionValues.editTopicName.id) { return FORCE_EDIT_OPTIONS; + } else if (permission === this.permissionValues.submit.id) { + return FORCE_SUBMIT_OPTIONS; } return []; }, diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html index 70f0edfd0d..3e3bcfbec4 100644 --- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html +++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html @@ -71,6 +71,16 @@ limitations under the License. value: true, }, ]; + const FORCE_SUBMIT_OPTIONS = [ + { + name: 'No Submit on Push', + value: false, + }, + { + name: 'Submit on Push', + value: true, + }, + ]; let permission = 'push'; assert.isTrue(element._computeForce(permission)); assert.equal(element._computeForceClass(permission), 'force'); @@ -82,6 +92,11 @@ limitations under the License. assert.deepEqual(element._computeForceOptions(permission), FORCE_EDIT_OPTIONS); permission = 'submit'; + assert.isTrue(element._computeForce(permission)); + assert.equal(element._computeForceClass(permission), 'force'); + assert.deepEqual(element._computeForceOptions(permission), + FORCE_SUBMIT_OPTIONS); + permission = 'editHashtags'; assert.isFalse(element._computeForce(permission)); assert.equal(element._computeForceClass(permission), ''); assert.deepEqual(element._computeForceOptions(permission), []); @@ -124,7 +139,7 @@ limitations under the License. {action: 'ALLOW', force: false}); permission = 'submit'; assert.deepEqual(element._getDefaultRuleValues(permission, label), - {action: 'ALLOW'}); + {action: 'ALLOW', force: false}); }); test('_setDefaultRuleValues', () => { @@ -210,7 +225,7 @@ limitations under the License. assert.equal(element.$.action.bindValue, element.rule.value.action); assert.isNotOk(Polymer.dom(element.root).querySelector('#labelMin')); assert.isNotOk(Polymer.dom(element.root).querySelector('#labelMax')); - assert.isFalse(element.$.force.classList.contains('force')); + assert.isTrue(element.$.force.classList.contains('force')); }); test('modify and cancel restores original values', () => {