Merge branch 'stable-2.14' into stable-2.15

* stable-2.14:
  Add new "Delete Changes" permission
  ChangeIT: Refactor tests for deletion of new changes

Change-Id: I6eda2f9901118fe7eadab9811258fd042e2349a0

Documentation/access-control.txt

@@ -841,6 +841,17 @@ right assigned).
 This category permits users to delete their own changes if they are not merged
 yet. This means only own changes that are open or abandoned can be deleted.
 
+[[category_delete_changes]]
+=== Delete Changes
+
+This category permits users to delete other users' changes if they are not merged
+yet. This means only changes that are open or abandoned can be deleted.
+
+Having this permission implies having the link:#category_delete_own_changes[
+Delete Own Changes] permission.
+
+Administrators may always delete changes without having this permission.
+
 [[category_edit_topic_name]]
 === Edit Topic Name
 

Documentation/user-review-ui.txt

@@ -258,7 +258,9 @@ Deletes the change.
 For open or abandoned changes, the `Delete Change` button will be available
 and if the user is the change owner and is granted the
 link:access-control.html#category_delete_own_changes[Delete Own Changes]
-permission or if they are an administrator.
+permission, if they are granted the
+link:access-control.html#category_delete_changes[Delete Changes] permission,
+or if they are an administrator.
 
 ** [[plugin-actions]]Further actions may be available if plugins are installed.
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -44,6 +44,7 @@ import static com.google.gerrit.reviewdb.server.ReviewDbUtil.unwrapDb;
 import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
+import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.server.project.Util.category;
 import static com.google.gerrit.server.project.Util.value;
@@ -89,6 +90,8 @@ import com.google.gerrit.extensions.api.changes.StarsInput;
 import com.google.gerrit.extensions.api.groups.GroupApi;
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.api.projects.ConfigInput;
+import com.google.gerrit.extensions.api.projects.ProjectApi;
+import com.google.gerrit.extensions.api.projects.ProjectInput;
 import com.google.gerrit.extensions.client.ChangeKind;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.Comment.Range;
@@ -927,12 +930,7 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Test
   public void deleteNewChangeAsAdmin() throws Exception {
-    PushOneCommit.Result changeResult = createChange();
-    String changeId = changeResult.getChangeId();
-
-    gApi.changes().id(changeId).delete();
-
-    assertThat(query(changeId)).isEmpty();
+    deleteChangeAsUser(admin, admin);
   }
 
   @Test
@@ -949,51 +947,79 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
-  @TestProjectInput(cloneAs = "user")
-  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
-    allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
-    deleteChangeAsUser();
+  public void deleteNewChangeAsUserWithDeleteChangesPermissionForGroup() throws Exception {
+    allow("refs/*", Permission.DELETE_CHANGES, REGISTERED_USERS);
+    deleteChangeAsUser(admin, user);
   }
 
   @Test
-  @TestProjectInput(cloneAs = "user")
-  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
-    allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
-    deleteChangeAsUser();
+  public void deleteNewChangeAsUserWithDeleteChangesPermissionForProjectOwners() throws Exception {
+    GroupApi groupApi = gApi.groups().create(name("delete-change"));
+    groupApi.addMembers("user");
+
+    ProjectInput in = new ProjectInput();
+    in.name = name("delete-change");
+    in.owners = Lists.newArrayListWithCapacity(1);
+    in.owners.add(groupApi.name());
+    in.createEmptyCommit = true;
+    ProjectApi api = gApi.projects().create(in);
+
+    Project.NameKey nameKey = new Project.NameKey(api.get().name);
+
+    ProjectConfig cfg = projectCache.checkedGet(nameKey).getConfig();
+    Util.allow(cfg, Permission.DELETE_CHANGES, PROJECT_OWNERS, "refs/*");
+    saveProjectConfig(nameKey, cfg);
+
+    deleteChangeAsUser(nameKey, admin, user);
   }
 
-  private void deleteChangeAsUser() throws Exception {
-    try {
-      PushOneCommit.Result changeResult =
-          pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
-      String changeId = changeResult.getChangeId();
-      int id = changeResult.getChange().getId().id;
-      RevCommit commit = changeResult.getCommit();
+  @Test
+  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
+    allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
+    deleteChangeAsUser(user, user);
+  }
 
-      setApiUser(user);
+  @Test
+  public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
+    allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
+    deleteChangeAsUser(user, user);
+  }
+
+  private void deleteChangeAsUser(TestAccount owner, TestAccount deleteAs) throws Exception {
+    deleteChangeAsUser(project, owner, deleteAs);
+  }
+
+  private void deleteChangeAsUser(
+      Project.NameKey projectName, TestAccount owner, TestAccount deleteAs) throws Exception {
+    try {
+      setApiUser(owner);
+      ChangeInput in = new ChangeInput();
+      in.project = projectName.get();
+      in.branch = "refs/heads/master";
+      in.subject = "test";
+      ChangeInfo changeInfo = gApi.changes().create(in).get();
+      String changeId = changeInfo.changeId;
+      int id = changeInfo._number;
+      String commit = changeInfo.currentRevision;
+
+      assertThat(gApi.changes().id(changeId).info().owner._accountId).isEqualTo(owner.id.get());
+
+      setApiUser(deleteAs);
       gApi.changes().id(changeId).delete();
 
       assertThat(query(changeId)).isEmpty();
 
       String ref = new Change.Id(id).toRefPrefix() + "1";
-      eventRecorder.assertRefUpdatedEvents(project.get(), ref, null, commit, commit, null);
+      eventRecorder.assertRefUpdatedEvents(projectName.get(), ref, null, commit, commit, null);
     } finally {
       removePermission(project, "refs/*", Permission.DELETE_OWN_CHANGES);
+      removePermission(project, "refs/*", Permission.DELETE_CHANGES);
     }
   }
 
   @Test
-  @TestProjectInput(cloneAs = "user")
   public void deleteNewChangeOfAnotherUserAsAdmin() throws Exception {
-    PushOneCommit.Result changeResult =
-        pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
-    changeResult.assertOkStatus();
-    String changeId = changeResult.getChangeId();
-
-    setApiUser(admin);
-    gApi.changes().id(changeId).delete();
-
-    assertThat(query(changeId)).isEmpty();
+    deleteChangeAsUser(user, admin);
   }
 
   @Test

gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java

@@ -27,6 +27,7 @@ public class Permission implements Comparable<Permission> {
   public static final String DELETE = "delete";
   public static final String CREATE_TAG = "createTag";
   public static final String CREATE_SIGNED_TAG = "createSignedTag";
+  public static final String DELETE_CHANGES = "deleteChanges";
   public static final String DELETE_OWN_CHANGES = "deleteOwnChanges";
   public static final String EDIT_HASHTAGS = "editHashtags";
   public static final String EDIT_ASSIGNEE = "editAssignee";
@@ -76,6 +77,7 @@ public class Permission implements Comparable<Permission> {
     NAMES_LC.add(EDIT_HASHTAGS.toLowerCase());
     NAMES_LC.add(EDIT_ASSIGNEE.toLowerCase());
     NAMES_LC.add(DELETE_OWN_CHANGES.toLowerCase());
+    NAMES_LC.add(DELETE_CHANGES.toLowerCase());
 
     LABEL_INDEX = NAMES_LC.indexOf(Permission.LABEL);
     LABEL_AS_INDEX = NAMES_LC.indexOf(Permission.LABEL_AS.toLowerCase());

gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java

@@ -156,8 +156,7 @@ class ChangeControl {
     switch (status) {
       case NEW:
       case ABANDONED:
-        return (isOwner() && getRefControl().canDeleteOwnChanges(isOwner()))
-            || getProjectControl().isAdmin();
+        return (getRefControl().canDeleteChanges(isOwner()) || getProjectControl().isAdmin());
       case MERGED:
       default:
         return false;

gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java

@@ -285,9 +285,10 @@ public class RefControl {
     return canPerform(Permission.VIEW_PRIVATE_CHANGES);
   }
 
-  /** @return true if this user can delete their own changes. */
-  boolean canDeleteOwnChanges(boolean isChangeOwner) {
-    return canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner);
+  /** @return true if this user can delete changes. */
+  boolean canDeleteChanges(boolean isChangeOwner) {
+    return canPerform(Permission.DELETE_CHANGES)
+        || (isChangeOwner && canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner));
   }
 
   /** @return true if this user can edit topic names. */