Allow editing of commit message from UI.

This change adds a small edit icon on the top right
corner of the commit message box on the change screen.

Clicking it lets the user edit the commit message and a
new patch set will be created.

Change-Id: I2c4d4a3ba520d4c12ade4bb3b47a55ce7f1b19df
This commit is contained in:
Gustaf Lundh
2012-11-16 14:59:02 -08:00
committed by Shawn O. Pearce
parent 5367b8bab5
commit be1636e870
13 changed files with 339 additions and 8 deletions

View File

@@ -29,6 +29,7 @@ public class ChangeDetail {
protected AccountInfoCache accounts;
protected boolean allowsAnonymous;
protected boolean canAbandon;
protected boolean canEditCommitMessage;
protected boolean canPublish;
protected boolean canRebase;
protected boolean canRestore;
@@ -77,6 +78,14 @@ public class ChangeDetail {
canAbandon = a;
}
public boolean canEditCommitMessage() {
return canEditCommitMessage;
}
public void setCanEditCommitMessage(final boolean a) {
canEditCommitMessage = a;
}
public boolean canPublish() {
return canPublish;
}

View File

@@ -29,6 +29,11 @@ public interface ChangeManageService extends RemoteJsonService {
@SignInRequired
void submit(PatchSet.Id patchSetId, AsyncCallback<ChangeDetail> callback);
@Audit
@SignInRequired
void createNewPatchSet(final PatchSet.Id patchSetId, final String newCommitMessage,
final AsyncCallback<ChangeDetail> callback);
@Audit
@SignInRequired
void revertChange(PatchSet.Id patchSetId, String message,

View File

@@ -129,6 +129,9 @@ public interface ChangeConstants extends Constants {
String headingRevertMessage();
String revertChangeTitle();
String headingEditCommitMessage();
String titleEditCommitMessage();
String buttonAbandonChangeBegin();
String buttonAbandonChangeSend();
String headingAbandonMessage();

View File

@@ -114,6 +114,9 @@ buttonRevertChangeSend = Revert Change
headingRevertMessage = Revert Commit Message:
revertChangeTitle = Code Review - Revert Merged Change
headingEditCommitMessage = Commit Message
titleEditCommitMessage = Create New Patch Set
buttonRestoreChangeBegin = Restore Change
restoreChangeTitle = Code Review - Restore Change
headingRestoreMessage = Restore Message:

View File

@@ -36,9 +36,11 @@ public class ChangeDescriptionBlock extends Composite {
initWidget(hp);
}
public void display(Change chg, Boolean starred, PatchSetInfo info,
public void display(Change chg, Boolean starred, Boolean canEditCommitMessage,
PatchSetInfo info,
final AccountInfoCache acc, SubmitTypeRecord submitTypeRecord) {
infoBlock.display(chg, acc, submitTypeRecord);
messageBlock.display(chg.getId(), starred, info.getMessage());
messageBlock.display(chg.currentPatchSetId(), starred,
canEditCommitMessage, info.getMessage());
}
}

View File

@@ -281,6 +281,7 @@ public class ChangeScreen extends Screen
descriptionBlock.display(detail.getChange(),
detail.isStarred(),
detail.canEditCommitMessage(),
detail.getCurrentPatchSetDetail().getInfo(),
detail.getAccounts(), detail.getSubmitTypeRecord());
dependsOn.display(detail.getDependsOn());

View File

@@ -17,16 +17,22 @@ package com.google.gerrit.client.changes;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.ui.ChangeLink;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.CommentedActionDialog;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.PreElement;
import com.google.gwt.dom.client.Style.Display;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.uibinder.client.UiBinder;
import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.HTMLPanel;
import com.google.gwt.user.client.ui.Image;
import com.google.gwt.user.client.ui.SimplePanel;
import com.google.gwtexpui.clippy.client.CopyableLabel;
import com.google.gwtexpui.globalkey.client.KeyCommandSet;
@@ -60,12 +66,13 @@ public class CommitMessageBlock extends Composite {
}
public void display(final String commitMessage) {
display(null, null, commitMessage);
display(null, null, false, commitMessage);
}
public void display(Change.Id changeId, Boolean starred, String commitMessage) {
public void display(final PatchSet.Id patchSetId,
Boolean starred, Boolean canEditCommitMessage, final String commitMessage) {
starPanel.clear();
Change.Id changeId = patchSetId.getParentKey();
if (changeId != null && starred != null && Gerrit.isSignedIn()) {
StarredChanges.Icon star = StarredChanges.createIcon(changeId, starred);
star.setStyleName(Gerrit.RESOURCES.css().changeScreenStarIcon());
@@ -79,7 +86,33 @@ public class CommitMessageBlock extends Composite {
permalinkPanel.clear();
if (changeId != null) {
permalinkPanel.add(new ChangeLink(Util.C.changePermalink(), changeId));
permalinkPanel.add(new CopyableLabel(ChangeLink.permalink(changeId), false));
permalinkPanel.add(new CopyableLabel(ChangeLink.permalink(changeId),
false));
if (canEditCommitMessage) {
final Image edit = new Image(Gerrit.RESOURCES.edit());
edit.addClickHandler(new ClickHandler() {
@Override
public void onClick(final ClickEvent event) {
new CommentedActionDialog<ChangeDetail>(Util.C
.titleEditCommitMessage(), Util.C.headingEditCommitMessage(),
new ChangeDetailCache.IgnoreErrorCallback() {}) {
{
message.setCharacterWidth(80);
message.setVisibleLines(20);
message.setText(commitMessage);
}
@Override
public void onSend() {
Util.MANAGE_SVC.createNewPatchSet(patchSetId, getMessageText(),
createCallback());
}
}.center();
}
});
permalinkPanel.add(edit);
}
}
String[] splitCommitMessage = commitMessage.split("\n", 2);

View File

@@ -274,7 +274,7 @@ public class PublishCommentScreen extends AccountScreen implements
private void display(final PatchSetPublishDetail r) {
setPageTitle(Util.M.publishComments(r.getChange().getKey().abbreviate(),
patchSetId.get()));
descBlock.display(r.getChange(), null, r.getPatchSetInfo(), r.getAccounts(),
descBlock.display(r.getChange(), null, false, r.getPatchSetInfo(), r.getAccounts(),
r.getSubmitTypeRecord());
if (r.getChange().getStatus().isOpen()) {

View File

@@ -154,6 +154,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
detail.setCanRevert(change.getStatus() == Change.Status.MERGED && control.canAddPatchSet());
detail.setCanEdit(control.getRefControl().canWrite());
detail.setCanEditCommitMessage(change.getStatus().isOpen() && control.canAddPatchSet());
detail.setCanEditTopicName(control.canEditTopicName());
List<SubmitRecord> submitRecords = control.getSubmitRecords(db, patch);

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.httpd.rpc.changedetail;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.ChangeManageService;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gwtjsonrpc.common.AsyncCallback;
import com.google.gwtjsonrpc.common.VoidResult;
import com.google.inject.Inject;
@@ -28,6 +29,7 @@ class ChangeManageServiceImpl implements ChangeManageService {
private final RevertChange.Factory revertChangeFactory;
private final PublishAction.Factory publishAction;
private final DeleteDraftChange.Factory deleteDraftChangeFactory;
private final EditCommitMessageHandler.Factory editCommitMessageHandlerFactory;
@Inject
ChangeManageServiceImpl(final SubmitAction.Factory patchSetAction,
@@ -35,13 +37,15 @@ class ChangeManageServiceImpl implements ChangeManageService {
final RestoreChangeHandler.Factory restoreChangeHandlerFactory,
final RevertChange.Factory revertChangeFactory,
final PublishAction.Factory publishAction,
final DeleteDraftChange.Factory deleteDraftChangeFactory) {
final DeleteDraftChange.Factory deleteDraftChangeFactory,
final EditCommitMessageHandler.Factory editCommitMessageHandler) {
this.submitAction = patchSetAction;
this.rebaseChangeFactory = rebaseChangeFactory;
this.restoreChangeHandlerFactory = restoreChangeHandlerFactory;
this.revertChangeFactory = revertChangeFactory;
this.publishAction = publishAction;
this.deleteDraftChangeFactory = deleteDraftChangeFactory;
this.editCommitMessageHandlerFactory = editCommitMessageHandler;
}
public void submit(final PatchSet.Id patchSetId,
@@ -73,4 +77,9 @@ class ChangeManageServiceImpl implements ChangeManageService {
final AsyncCallback<VoidResult> callback) {
deleteDraftChangeFactory.create(patchSetId).to(callback);
}
public void createNewPatchSet(Id patchSetId, String message,
AsyncCallback<ChangeDetail> callback) {
editCommitMessageHandlerFactory.create(patchSetId, message).to(callback);
}
}

View File

@@ -28,6 +28,7 @@ public class ChangeModule extends RpcServletModule {
install(new FactoryModule() {
@Override
protected void configure() {
factory(EditCommitMessageHandler.Factory.class);
factory(RestoreChangeHandler.Factory.class);
factory(RevertChange.Factory.class);
factory(RebaseChangeHandler.Factory.class);

View File

@@ -0,0 +1,112 @@
// Copyright (C) 2012 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.httpd.rpc.changedetail;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.IOException;
import javax.annotation.Nullable;
class EditCommitMessageHandler extends Handler<ChangeDetail> {
interface Factory {
EditCommitMessageHandler create(PatchSet.Id patchSetId, String message);
}
private final ChangeControl.Factory changeControlFactory;
private final ReviewDb db;
private final IdentifiedUser currentUser;
private final ChangeDetailFactory.Factory changeDetailFactory;
private final GitReferenceUpdated replication;
private final PatchSet.Id patchSetId;
@Nullable
private final String message;
private final ChangeHooks hooks;
private final GitRepositoryManager gitManager;
private final PatchSetInfoFactory patchSetInfoFactory;
private final PersonIdent myIdent;
@Inject
EditCommitMessageHandler(final ChangeControl.Factory changeControlFactory,
final ReviewDb db, final IdentifiedUser currentUser,
final ChangeDetailFactory.Factory changeDetailFactory,
@Assisted final PatchSet.Id patchSetId,
@Assisted @Nullable final String message, final ChangeHooks hooks,
final GitRepositoryManager gitManager,
final PatchSetInfoFactory patchSetInfoFactory,
final GitReferenceUpdated replication,
@GerritPersonIdent final PersonIdent myIdent) {
this.changeControlFactory = changeControlFactory;
this.db = db;
this.currentUser = currentUser;
this.changeDetailFactory = changeDetailFactory;
this.patchSetId = patchSetId;
this.message = message;
this.hooks = hooks;
this.gitManager = gitManager;
this.patchSetInfoFactory = patchSetInfoFactory;
this.replication = replication;
this.myIdent = myIdent;
}
@Override
public ChangeDetail call() throws NoSuchChangeException, OrmException,
EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException,
MissingObjectException, IncorrectObjectTypeException, IOException,
InvalidChangeOperationException {
final Change.Id changeId = patchSetId.getParentKey();
final ChangeControl control = changeControlFactory.validateFor(changeId);
if (!control.canAddPatchSet()) {
throw new InvalidChangeOperationException(
"Not allowed to add new Patch Sets to: " + changeId.toString());
}
ChangeUtil.editCommitMessage(patchSetId, currentUser, message, db,
hooks, gitManager, patchSetInfoFactory, replication, myIdent);
return changeDetailFactory.create(changeId).call();
}
}

View File

@@ -19,6 +19,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetAncestor;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.TrackingId;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -31,7 +32,10 @@ import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.mail.ReplyToChangeSender;
import com.google.gerrit.server.mail.RevertedSender;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmException;
@@ -293,6 +297,154 @@ public class ChangeUtil {
}
}
public static Change.Id editCommitMessage(final PatchSet.Id patchSetId,
final IdentifiedUser user, final String message, final ReviewDb db,
final ChangeHooks hooks, GitRepositoryManager gitManager,
final PatchSetInfoFactory patchSetInfoFactory,
final GitReferenceUpdated replication, PersonIdent myIdent)
throws NoSuchChangeException, EmailException, OrmException,
MissingObjectException, IncorrectObjectTypeException, IOException,
InvalidChangeOperationException, PatchSetInfoNotAvailableException {
final Change.Id changeId = patchSetId.getParentKey();
final PatchSet patch = db.patchSets().get(patchSetId);
if (patch == null) {
throw new NoSuchChangeException(changeId);
}
if (message == null || message.length() == 0) {
throw new InvalidChangeOperationException("The commit message cannot be empty");
}
final Repository git;
try {
git = gitManager.openRepository(db.changes().get(changeId).getProject());
} catch (RepositoryNotFoundException e) {
throw new NoSuchChangeException(changeId, e);
}
try {
final RevWalk revWalk = new RevWalk(git);
try {
Change change = db.changes().get(changeId);
RevCommit commit =
revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get()));
PersonIdent authorIdent =
user.newCommitterIdent(myIdent.getWhen(), myIdent.getTimeZone());
CommitBuilder commitBuilder = new CommitBuilder();
commitBuilder.addParentId(commit);
commitBuilder.setTreeId(commit.getTree());
commitBuilder.setAuthor(authorIdent);
commitBuilder.setCommitter(myIdent);
commitBuilder.setMessage(message);
RevCommit newCommit;
final ObjectInserter oi = git.newObjectInserter();
try {
ObjectId id = oi.insert(commitBuilder);
oi.flush();
newCommit = revWalk.parseCommit(id);
} finally {
oi.release();
}
change.nextPatchSetId();
final PatchSet originalPS = db.patchSets().get(patchSetId);
final PatchSet newPatchSet = new PatchSet(change.currPatchSetId());
newPatchSet.setCreatedOn(change.getCreatedOn());
newPatchSet.setUploader(change.getOwner());
newPatchSet.setRevision(new RevId(newCommit.name()));
newPatchSet.setDraft(originalPS.isDraft());
final PatchSetInfo info =
patchSetInfoFactory.get(newCommit, newPatchSet.getId());
final RefUpdate ru = git.updateRef(newPatchSet.getRefName());
ru.setExpectedOldObjectId(ObjectId.zeroId());
ru.setNewObjectId(newCommit);
ru.disableRefLog();
if (ru.update(revWalk) != RefUpdate.Result.NEW) {
throw new IOException(String.format(
"Failed to create ref %s in %s: %s", newPatchSet.getRefName(),
change.getDest().getParentKey().get(), ru.getResult()));
}
replication.fire(change.getProject(), ru.getName());
db.changes().beginTransaction(change.getId());
try {
Change updatedChange =
db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
@Override
public Change update(Change change) {
if (change.getStatus().isOpen()) {
change.updateNumberOfPatchSets(newPatchSet.getPatchSetId());
return change;
} else {
return null;
}
}
});
if (updatedChange != null) {
change = updatedChange;
} else {
throw new InvalidChangeOperationException(String.format(
"Change %s is closed", change.getId()));
}
ChangeUtil.insertAncestors(db, newPatchSet.getId(), commit);
db.patchSets().insert(Collections.singleton(newPatchSet));
updatedChange =
db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
@Override
public Change update(Change change) {
if (change.getStatus().isClosed()) {
return null;
}
if (!change.currentPatchSetId().equals(patchSetId)) {
return null;
}
if (change.getStatus() != Change.Status.DRAFT) {
change.setStatus(Change.Status.NEW);
}
change.setLastSha1MergeTested(null);
change.setCurrentPatchSet(info);
ChangeUtil.updated(change);
return change;
}
});
if (updatedChange != null) {
change = updatedChange;
} else {
throw new InvalidChangeOperationException(String.format(
"Change %s was modified", change.getId()));
}
final ChangeMessage cmsg =
new ChangeMessage(new ChangeMessage.Key(changeId,
ChangeUtil.messageUUID(db)), user.getAccountId(), patchSetId);
final String msg = "Patch Set " + newPatchSet.getPatchSetId() + ": Commit message was updated";
cmsg.setMessage(msg);
db.changeMessages().insert(Collections.singleton(cmsg));
db.commit();
} finally {
db.rollback();
}
hooks.doPatchsetCreatedHook(change, newPatchSet, db);
return change.getId();
} finally {
revWalk.release();
}
} finally {
git.close();
}
}
public static void deleteDraftChange(final PatchSet.Id patchSetId,
GitRepositoryManager gitManager,
final GitReferenceUpdated replication, final ReviewDb db)