Make the ability to edit topic names a grantable permission
Currently any user who can upload patchsets on a change is able to
edit the change's topic name.
This is a bit too permissive. Many companies allow all developers to
upload changes on any project, but they may not necessarily want them
all to be able to edit topic names.
Add a new permission that can be granted to allow users to edit topic
names. Make the edit dialog only available to users who explicitly
have the permission, the project owners, the branch owner, and site
administrators.
Change-Id: Ife02370c664b2d206d731a495988a06d3123c54d
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 2c22c45..c768b34 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -771,6 +771,18 @@
assigned).
+[[category_edit_topic_name]]
+Edit Topic Name
+~~~~~~~~~~~~~~~
+
+This category permits users to edit the topic name of a change that
+is uploaded for review.
+
+The change owner, branch owners, project owners, and site administrators
+can always edit the topic name (even without having the `Edit Topic Name`
+access right assigned).
+
+
[[category_makeoneup]]
Your Category Here
~~~~~~~~~~~~~~~~~~
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java
index 529283b..f8fdaa3 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java
@@ -48,6 +48,7 @@
protected PatchSet.Id currentPatchSetId;
protected PatchSetDetail currentDetail;
protected boolean canEdit;
+ protected boolean canEditTopicName;
public ChangeDetail() {
}
@@ -124,6 +125,14 @@
canDeleteDraft = a;
}
+ public boolean canEditTopicName() {
+ return canEditTopicName;
+ }
+
+ public void setCanEditTopicName(boolean a) {
+ canEditTopicName = a;
+ }
+
public Change getChange() {
return change;
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
index be4da4e..7ff0273e 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
@@ -23,6 +23,7 @@
public class Permission implements Comparable<Permission> {
public static final String ABANDON = "abandon";
public static final String CREATE = "create";
+ public static final String EDIT_TOPIC_NAME = "editTopicName";
public static final String FORGE_AUTHOR = "forgeAuthor";
public static final String FORGE_COMMITTER = "forgeCommitter";
public static final String FORGE_SERVER = "forgeServerAsCommitter";
@@ -57,6 +58,7 @@
NAMES_LC.add(REMOVE_REVIEWER.toLowerCase());
NAMES_LC.add(SUBMIT.toLowerCase());
NAMES_LC.add(VIEW_DRAFTS.toLowerCase());
+ NAMES_LC.add(EDIT_TOPIC_NAME.toLowerCase());
labelIndex = NAMES_LC.indexOf(Permission.LABEL);
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
index c0bf818..1c87c71 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
@@ -82,7 +82,10 @@
GIT_ERROR,
/** The destination branch does not exist */
- DEST_BRANCH_NOT_FOUND
+ DEST_BRANCH_NOT_FOUND,
+
+ /** Not permitted to edit the topic name */
+ EDIT_TOPIC_NAME_NOT_PERMITTED
}
protected Type type;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
index 08c6cce..c353b10 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
@@ -105,6 +105,7 @@
permissionNames = \
abandon, \
create, \
+ editTopicName, \
forgeAuthor, \
forgeCommitter, \
forgeServerAsCommitter, \
@@ -119,6 +120,7 @@
viewDrafts
abandon = Abandon
create = Create Reference
+editTopicName = Edit Topic Name
forgeAuthor = Forge Author Identity
forgeCommitter = Forge Committer Identity
forgeServerAsCommitter = Forge Server Identity
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
index 2959af1..e0c77f7 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
@@ -143,15 +143,20 @@
fp.add(new BranchLink(chg.getTopic(), chg.getProject(), chg.getStatus(),
dst.get(), chg.getTopic()));
- final Image edit = new Image(Gerrit.RESOURCES.edit());
- edit.addClickHandler(new ClickHandler() {
- @Override
- public void onClick(final ClickEvent event) {
- new AlterTopicDialog(chg).center();
- }
- });
- edit.addStyleName(Gerrit.RESOURCES.css().changeInfoBlockEdit());
- fp.add(edit);
+ ChangeDetailCache detailCache = ChangeCache.get(chg.getId()).getChangeDetailCache();
+ ChangeDetail changeDetail = detailCache.get();
+
+ if (changeDetail.canEditTopicName()) {
+ final Image edit = new Image(Gerrit.RESOURCES.edit());
+ edit.addClickHandler(new ClickHandler() {
+ @Override
+ public void onClick(final ClickEvent event) {
+ new AlterTopicDialog(chg).center();
+ }
+ });
+ edit.addStyleName(Gerrit.RESOURCES.css().changeInfoBlockEdit());
+ fp.add(edit);
+ }
return fp;
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
index d23b1bc..7150b67 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
@@ -154,6 +154,7 @@
detail.setCanRevert(change.getStatus() == Change.Status.MERGED && control.canAddPatchSet());
detail.setCanEdit(control.getRefControl().canWrite());
+ detail.setCanEditTopicName(control.canEditTopicName());
List<SubmitRecord> submitRecords = control.getSubmitRecords(db, patch);
for (SubmitRecord rec : submitRecords) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java
index 4ae2e8a..9c4bc21 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.changedetail;
+import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -24,7 +25,6 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gwtjsonrpc.common.VoidResult;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -35,7 +35,7 @@
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
-public class AlterTopic implements Callable<VoidResult> {
+public class AlterTopic implements Callable<ReviewResult> {
private final ChangeControl.Factory changeControlFactory;
private final ReviewDb db;
@@ -77,12 +77,20 @@
}
@Override
- public VoidResult call() throws EmailException,
+ public ReviewResult call() throws EmailException,
InvalidChangeOperationException, NoSuchChangeException, OrmException {
final ChangeControl control = changeControlFactory.validateFor(changeId);
+ final ReviewResult result = new ReviewResult();
+ result.setChangeId(changeId);
+
if (!control.canAddPatchSet()) {
throw new NoSuchChangeException(changeId);
}
+ if (!control.canEditTopicName()) {
+ result.addError(new ReviewResult.Error(
+ ReviewResult.Error.Type.EDIT_TOPIC_NAME_NOT_PERMITTED));
+ return result;
+ }
final Change change = db.changes().get(changeId);
if (!change.getTopic().equals(topic)) {
@@ -114,6 +122,6 @@
db.changeMessages().insert(Collections.singleton(cmsg));
}
- return VoidResult.INSTANCE;
+ return result;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 4c48a71..2ab3fde 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -289,6 +289,16 @@
return false;
}
+ /** Can this user edit the topic name? */
+ public boolean canEditTopicName() {
+ return isOwner() // owner (aka creator) of the change can edit topic
+ || getRefControl().isOwner() // branch owner can edit topic
+ || getProjectControl().isOwner() // project owner can edit topic
+ || getCurrentUser().getCapabilities().canAdministrateServer() // site administers are god
+ || getRefControl().canEditTopicName() // user can edit topic on a specific ref
+ ;
+ }
+
public List<SubmitRecord> getSubmitRecords(ReviewDb db, PatchSet patchSet) {
return canSubmit(db, patchSet, null, false, true);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 7f3c5d2..2175262 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -332,6 +332,10 @@
return canPerform(Permission.VIEW_DRAFTS);
}
+ public boolean canEditTopicName() {
+ return canPerform(Permission.EDIT_TOPIC_NAME);
+ }
+
/** All value ranges of any allowed label permission. */
public List<PermissionRange> getLabelRanges() {
List<PermissionRange> r = new ArrayList<PermissionRange>();
