From 60400e2bc11ed48641580ca35ba342c692580fae Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 18 Aug 2013 16:25:40 -0700 Subject: [PATCH] ChangeScreen2: allow edit commit message Add new "Edit Message" button in change header and bind it to "e" shortcut. Add a new UiAction as REST API endpoint to create a new revision with updated commit message. Reload the screen on success. Change-Id: Ic555f16efba4783eca9684340b27f9d58b1482bb --- .../google/gerrit/client/change/Actions.java | 2 +- .../gerrit/client/change/ChangeScreen2.java | 29 ++++ .../gerrit/client/change/ChangeScreen2.ui.xml | 7 + .../client/change/EditMessageAction.java | 80 +++++++++++ .../gerrit/client/change/EditMessageBox.java | 102 ++++++++++++++ .../client/change/EditMessageBox.ui.xml | 51 +++++++ .../gerrit/client/change/ReplyBox.ui.xml | 11 +- .../gerrit/client/change/Resources.java | 1 + .../google/gerrit/client/change/common.css | 5 + .../gerrit/client/changes/ChangeApi.java | 8 ++ .../client/changes/ChangeConstants.java | 1 + .../client/changes/ChangeConstants.properties | 2 + .../gerrit/server/change/EditMessage.java | 126 ++++++++++++++++++ .../google/gerrit/server/change/Module.java | 1 + 14 files changed, 417 insertions(+), 9 deletions(-) create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageAction.java create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.java create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.ui.xml create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/EditMessage.java diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java index dbb5ce2473..7531e25fa7 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java @@ -35,7 +35,7 @@ import java.util.TreeSet; class Actions extends Composite { private static final String[] CORE = { "abandon", "restore", "revert", "topic", - "cherrypick", "submit", "rebase"}; + "cherrypick", "submit", "rebase", "message"}; interface Binder extends UiBinder {} private static Binder uiBinder = GWT.create(Binder.class); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java index 43f9b6d92a..a7f0ef8aeb 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java @@ -19,6 +19,7 @@ import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.account.AccountInfo; import com.google.gerrit.client.changes.ChangeApi; import com.google.gerrit.client.changes.ChangeInfo; +import com.google.gerrit.client.changes.ChangeInfo.ActionInfo; import com.google.gerrit.client.changes.ChangeInfo.ApprovalInfo; import com.google.gerrit.client.changes.ChangeInfo.CommitInfo; import com.google.gerrit.client.changes.ChangeInfo.LabelInfo; @@ -148,8 +149,10 @@ public class ChangeScreen2 extends Screen { @UiField Button reply; @UiField Button expandAll; @UiField Button collapseAll; + @UiField Button editMessage; @UiField QuickApprove quickApprove; private ReplyAction replyAction; + private EditMessageAction editMessageAction; public ChangeScreen2(Change.Id changeId, String revision, boolean openReplyBox) { this.changeId = changeId; @@ -241,6 +244,26 @@ public class ChangeScreen2 extends Screen { } } + private void initEditMessageAction() { + NativeMap actions = changeInfo.revision(revision).actions(); + if (actions != null && actions.containsKey("message")) { + editMessage.setVisible(true); + editMessageAction = new EditMessageAction( + changeInfo.legacy_id(), + revision, + changeInfo.revision(revision).commit().message(), + style, + editMessage, + reply); + keysAction.add(new KeyCommand(0, 'e', Util.C.keyEditMessage()) { + @Override + public void onKeyPress(KeyPressEvent event) { + editMessageAction.onEdit(); + } + }); + } + } + @Override public void registerKeys() { super.registerKeys(); @@ -325,6 +348,11 @@ public class ChangeScreen2 extends Screen { } } + @UiHandler("editMessage") + void onEditMessage(ClickEvent e) { + editMessageAction.onEdit(); + } + @UiHandler("expandAll") void onExpandAll(ClickEvent e) { int n = history.getWidgetCount(); @@ -566,6 +594,7 @@ public class ChangeScreen2 extends Screen { quickApprove.set(info, revision); if (Gerrit.isSignedIn()) { + initEditMessageAction(); replyAction = new ReplyAction(info, revision, style, reply); if (topic.canEdit()) { keysAction.add(new KeyCommand(0, 't', Util.C.keyEditTopic()) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml index 2a356b0b9a..0459ec1a0d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml @@ -222,6 +222,13 @@ limitations under the License. title='Apply score with one click'> + + +
Edit Message
+
Revision diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageAction.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageAction.java new file mode 100644 index 0000000000..a0f7c9d6d9 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageAction.java @@ -0,0 +1,80 @@ +// Copyright (C) 2013 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.client.change; + +import com.google.gerrit.reviewdb.client.Change; +import com.google.gwt.event.logical.shared.CloseEvent; +import com.google.gwt.event.logical.shared.CloseHandler; +import com.google.gwt.user.client.ui.PopupPanel; +import com.google.gwt.user.client.ui.Widget; +import com.google.gwtexpui.globalkey.client.GlobalKey; +import com.google.gwtexpui.user.client.PluginSafePopupPanel; + +class EditMessageAction { + private final Change.Id changeId; + private final String revision; + private final String originalMessage; + private final ChangeScreen2.Style style; + private final Widget editMessageButton; + private final Widget replyButton; + + private EditMessageBox editBox; + private PopupPanel popup; + + EditMessageAction( + Change.Id changeId, + String revision, + String originalMessage, + ChangeScreen2.Style style, + Widget editButton, + Widget replyButton) { + this.changeId = changeId; + this.revision = revision; + this.originalMessage = originalMessage; + this.style = style; + this.editMessageButton = editButton; + this.replyButton = replyButton; + } + + void onEdit() { + if (popup != null) { + popup.hide(); + return; + } + + if (editBox == null) { + editBox = new EditMessageBox( + changeId, + revision, + originalMessage); + } + + final PluginSafePopupPanel p = new PluginSafePopupPanel(true); + p.setStyleName(style.replyBox()); + p.addAutoHidePartner(editMessageButton.getElement()); + p.addCloseHandler(new CloseHandler() { + @Override + public void onClose(CloseEvent event) { + if (popup == p) { + popup = null; + } + } + }); + p.add(editBox); + p.showRelativeTo(replyButton); + GlobalKey.dialog(p); + popup = p; + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.java new file mode 100644 index 0000000000..5ddd21ba78 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.java @@ -0,0 +1,102 @@ +// Copyright (C) 2013 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.client.change; + +import com.google.gerrit.client.Gerrit; +import com.google.gerrit.client.changes.ChangeApi; +import com.google.gerrit.client.rpc.GerritCallback; +import com.google.gerrit.client.ui.TextBoxChangeListener; +import com.google.gerrit.common.PageLinks; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gwt.core.client.GWT; +import com.google.gwt.core.client.JavaScriptObject; +import com.google.gwt.core.client.Scheduler; +import com.google.gwt.core.client.Scheduler.ScheduledCommand; +import com.google.gwt.event.dom.client.ClickEvent; +import com.google.gwt.uibinder.client.UiBinder; +import com.google.gwt.uibinder.client.UiField; +import com.google.gwt.uibinder.client.UiHandler; +import com.google.gwt.user.client.ui.Button; +import com.google.gwt.user.client.ui.Composite; +import com.google.gwt.user.client.ui.HTMLPanel; +import com.google.gwt.user.client.ui.PopupPanel; +import com.google.gwt.user.client.ui.Widget; +import com.google.gwtexpui.globalkey.client.NpTextArea; + +class EditMessageBox extends Composite { + interface Binder extends UiBinder {} + private static final Binder uiBinder = GWT.create(Binder.class); + + private final Change.Id changeId; + private final String revision; + private String originalMessage; + + @UiField NpTextArea message; + @UiField Button save; + @UiField Button cancel; + + EditMessageBox( + Change.Id changeId, + String revision, + String msg) { + this.changeId = changeId; + this.revision = revision; + this.originalMessage = msg.trim(); + initWidget(uiBinder.createAndBindUi(this)); + new TextBoxChangeListener(message) { + public void onTextChanged(String newText) { + save.setEnabled(!newText.trim() + .equals(originalMessage)); + } + }; + } + + @Override + protected void onLoad() { + message.setText(originalMessage); + save.setEnabled(false); + Scheduler.get().scheduleDeferred(new ScheduledCommand() { + @Override + public void execute() { + message.setFocus(true); + }}); + } + + @UiHandler("save") + void onSave(ClickEvent e) { + ChangeApi.message(changeId.get(), revision, message.getText().trim(), + new GerritCallback() { + @Override + public void onSuccess(JavaScriptObject msg) { + Gerrit.display(PageLinks.toChange2(changeId)); + hide(); + }; + }); + } + + @UiHandler("cancel") + void onCancel(ClickEvent e) { + hide(); + } + + private void hide() { + for (Widget w = getParent(); w != null; w = w.getParent()) { + if (w instanceof PopupPanel) { + ((PopupPanel) w).hide(); + break; + } + } + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.ui.xml new file mode 100644 index 0000000000..118409ed48 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/EditMessageBox.ui.xml @@ -0,0 +1,51 @@ + + + + + + .commitMessage { + background-color: white; + font-family: monospace; + } + .cancel { float: right; } + + +
+ +
+
+ + +
Save
+ + +
Cancel
+ +
+ + diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.ui.xml index d17f48d1d4..afa9e86c9a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.ui.xml @@ -26,11 +26,6 @@ limitations under the License. max-height: 260px; } - .section { - padding: 5px 5px; - border-bottom: 1px solid #b8b8b8; - } - .label_name { font-weight: bold; text-align: left; @@ -46,16 +41,16 @@ limitations under the License. } -
+
-
+
-
+
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Resources.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Resources.java index 3f6cfe6d1c..76cbc6ad49 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Resources.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Resources.java @@ -33,5 +33,6 @@ public interface Resources extends ClientBundle { String button(); String popup(); String popupContent(); + String section(); } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/common.css b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/common.css index 725985c2bb..3dde21a6ee 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/common.css +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/common.css @@ -49,3 +49,8 @@ height: 10px; line-height: 10px; } + +.section { + padding: 5px 5px; + border-bottom: 1px solid #b8b8b8; +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java index bf8758ab4a..c292154066 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java @@ -102,6 +102,14 @@ public class ChangeApi { call(id, commit, "cherrypick").post(cherryPickInput, cb); } + /** Edit commit message for specific revision of a change. */ + public static void message(int id, String commit, String message, + AsyncCallback cb) { + CherryPickInput input = CherryPickInput.create(); + input.setMessage(message); + call(id, commit, "message").post(input, cb); + } + /** Submit a specific revision of a change. */ public static void submit(int id, String commit, AsyncCallback cb) { SubmitInput in = SubmitInput.create(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java index d1fcf605be..f0a9dc43fd 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java @@ -58,6 +58,7 @@ public interface ChangeConstants extends Constants { String keyReload(); String keyPublishComments(); String keyEditTopic(); + String keyEditMessage(); String patchTableColumnName(); String patchTableColumnComments(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties index eed019b957..c39db2186e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties @@ -38,6 +38,8 @@ nextPatchSet = Next patch set keyReload = Reload change keyPublishComments = Review and publish comments keyEditTopic = Edit change topic +keyEditMessage = Edit commit message + patchTableColumnName = File Path patchTableColumnComments = Comments diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/EditMessage.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/EditMessage.java new file mode 100644 index 0000000000..a634b7ccd6 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/EditMessage.java @@ -0,0 +1,126 @@ +// Copyright (C) 2013 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.server.change; + +import com.google.common.base.Strings; +import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.DefaultInput; +import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.extensions.webui.UiAction; +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.change.ChangeJson.ChangeInfo; +import com.google.gerrit.server.change.EditMessage.Input; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.mail.CommitMessageEditedSender; +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.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; + +import java.io.IOException; + +class EditMessage implements RestModifyView, + UiAction { + + private final Provider dbProvider; + private final CommitMessageEditedSender.Factory commitMessageEditedSenderFactory; + private final GitRepositoryManager gitManager; + private final PersonIdent myIdent; + private final PatchSetInserter.Factory patchSetInserterFactory; + private final ChangeJson json; + + static class Input { + @DefaultInput + String message; + } + + @Inject + EditMessage(final Provider dbProvider, + final CommitMessageEditedSender.Factory commitMessageEditedSenderFactory, + final GitRepositoryManager gitManager, + final PatchSetInserter.Factory patchSetInserterFactory, + @GerritPersonIdent final PersonIdent myIdent, + ChangeJson json) { + this.dbProvider = dbProvider; + this.commitMessageEditedSenderFactory = commitMessageEditedSenderFactory; + this.gitManager = gitManager; + this.myIdent = myIdent; + this.patchSetInserterFactory = patchSetInserterFactory; + this.json = json; + } + + @Override + public ChangeInfo apply(RevisionResource rsrc, Input input) + throws BadRequestException, ResourceConflictException, EmailException, + OrmException, ResourceNotFoundException, IOException { + if (Strings.isNullOrEmpty(input.message)) { + throw new BadRequestException("message must be non-empty"); + } + + final Repository git; + try { + git = gitManager.openRepository(rsrc.getChange().getProject()); + } catch (RepositoryNotFoundException e) { + throw new ResourceNotFoundException(e.getMessage()); + } + + try { + return json.format(ChangeUtil.editCommitMessage( + rsrc.getPatchSet().getId(), + rsrc.getControl().getRefControl(), + (IdentifiedUser) rsrc.getControl().getCurrentUser(), + input.message, dbProvider.get(), + commitMessageEditedSenderFactory, git, myIdent, + patchSetInserterFactory)); + } catch (InvalidChangeOperationException e) { + throw new BadRequestException(e.getMessage()); + } catch (MissingObjectException e) { + throw new ResourceConflictException(e.getMessage()); + } catch (IncorrectObjectTypeException e) { + throw new ResourceConflictException(e.getMessage()); + } catch (PatchSetInfoNotAvailableException e) { + throw new ResourceConflictException(e.getMessage()); + } catch (NoSuchChangeException e) { + throw new ResourceNotFoundException(); + } finally { + git.close(); + } + } + + @Override + public UiAction.Description getDescription(RevisionResource resource) { + PatchSet.Id current = resource.getChange().currentPatchSetId(); + return new UiAction.Description() + .setLabel("Edit commit message") + .setVisible(resource.getChange().getStatus().isOpen() + && resource.getPatchSet().getId().equals(current) + && resource.getControl().canAddPatchSet()); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index 814adde531..5f83f8bd11 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -69,6 +69,7 @@ public class Module extends RestApiModule { post(REVISION_KIND, "review").to(PostReview.class); post(REVISION_KIND, "submit").to(Submit.class); post(REVISION_KIND, "rebase").to(Rebase.class); + post(REVISION_KIND, "message").to(EditMessage.class); get(REVISION_KIND, "patch").to(GetPatch.class); get(REVISION_KIND, "submit_type").to(TestSubmitType.Get.class); post(REVISION_KIND, "test.submit_rule").to(TestSubmitRule.class);