From 0cf04a7f65a005f25dda7e129b288bb3d2ab4a6f Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 22 Jul 2013 10:18:24 -0700 Subject: [PATCH] ChangeScreen2: Display a list of related commits Instead of just showing the immediate parent(s) and immediate first child of the current revision, display the entire topic lineage back to the merge base with the branch, and all changes that build on this change. Display in a scroll pane to the right of the commit info. This makes navigation within a topic much easier, at a glance the reviewer can see where this change fits in and what changes come before and after it in the history. I am not sure about the result structure returned by /related. It is sufficient for this use case but may be too verbose and subject to changes as this feature is iterated on in the web UI. Documentation for /related is omitted until the project has settled on and can commit to an output format. Change-Id: I9b9e63a02af1c762fcad32cfef541af0b1fa9114 --- .../gerrit/client/change/ChangeScreen2.java | 7 + .../gerrit/client/change/ChangeScreen2.ui.xml | 9 +- .../gerrit/client/change/Constants.java | 21 ++ .../gerrit/client/change/Constants.properties | 3 + .../gerrit/client/change/FileTable.java | 2 +- .../gerrit/client/change/RelatedChanges.java | 338 ++++++++++++++++++ .../client/change/RelatedChanges.ui.xml | 41 +++ .../gerrit/client/change/Resources.java | 1 + .../server/PatchSetAncestorAccess.java | 4 + .../gerrit/server/change/GetRelated.java | 292 +++++++++++++++ .../google/gerrit/server/change/Module.java | 1 + 11 files changed, 717 insertions(+), 2 deletions(-) create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.java create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.ui.xml create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java 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 fb5d0f553e..21a236b337 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 @@ -127,6 +127,7 @@ public class ChangeScreen2 extends Screen { @UiField ListBox revisionList; @UiField Labels labels; @UiField CommitBox commit; + @UiField RelatedChanges related; @UiField FileTable files; @UiField FlowPanel history; @@ -210,12 +211,17 @@ public class ChangeScreen2 extends Screen { keys.add(GlobalKey.add(this, keysNavigation)); keys.add(GlobalKey.add(this, keysAction)); files.registerKeys(); + related.registerKeys(); } @Override public void onShowView() { super.onShowView(); + related.setMaxHeight(commit.getElement() + .getParentElement() + .getOffsetHeight()); + String prior = Gerrit.getPriorView(); if (prior != null && prior.startsWith("/c/")) { scrollToPath(prior.substring(3)); @@ -427,6 +433,7 @@ public class ChangeScreen2 extends Screen { reload.set(info); topic.set(info); commit.set(commentLinkProcessor, info, revision); + related.set(info, revision); quickApprove.set(info, revision); if (Gerrit.isSignedIn()) { 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 e0c56bb18b..0df9ff5e15 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 @@ -131,10 +131,13 @@ limitations under the License. color: red; } - .commitColumn { + .commitColumn, .related { padding: 0; vertical-align: top; } + .commitColumn { + width: 600px; + } .labels { border-spacing: 0; @@ -272,6 +275,10 @@ limitations under the License. + + + + diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java new file mode 100644 index 0000000000..e7b5d47d9c --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java @@ -0,0 +1,21 @@ +// 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; + +interface Constants extends com.google.gwt.i18n.client.Constants { + String previousChange(); + String nextChange(); + String openChange(); +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties new file mode 100644 index 0000000000..f5971b6d7f --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties @@ -0,0 +1,3 @@ +previousChange = Previous related change +nextChange = Next related change +openChange = Open related change diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java index c5005ef624..f2955c4c4d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java @@ -55,7 +55,7 @@ import java.util.Collections; import java.util.Comparator; class FileTable extends FlowPanel { - private static final FileTableResources R = GWT + static final FileTableResources R = GWT .create(FileTableResources.class); interface FileTableResources extends ClientBundle { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.java new file mode 100644 index 0000000000..c4c9e0937d --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.java @@ -0,0 +1,338 @@ +// 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.GitwebLink; +import com.google.gerrit.client.changes.ChangeApi; +import com.google.gerrit.client.changes.ChangeInfo; +import com.google.gerrit.client.changes.ChangeInfo.CommitInfo; +import com.google.gerrit.client.ui.NavigationTable; +import com.google.gerrit.common.PageLinks; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gwt.core.client.GWT; +import com.google.gwt.core.client.JavaScriptObject; +import com.google.gwt.core.client.JsArray; +import com.google.gwt.core.client.Scheduler; +import com.google.gwt.core.client.Scheduler.RepeatingCommand; +import com.google.gwt.dom.client.Element; +import com.google.gwt.dom.client.NativeEvent; +import com.google.gwt.resources.client.CssResource; +import com.google.gwt.uibinder.client.UiBinder; +import com.google.gwt.uibinder.client.UiField; +import com.google.gwt.user.client.DOM; +import com.google.gwt.user.client.Event; +import com.google.gwt.user.client.EventListener; +import com.google.gwt.user.client.Window; +import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.ui.Composite; +import com.google.gwt.user.client.ui.HTMLPanel; +import com.google.gwt.user.client.ui.ScrollPanel; +import com.google.gwt.user.client.ui.UIObject; +import com.google.gwt.user.client.ui.impl.HyperlinkImpl; +import com.google.gwtexpui.progress.client.ProgressBar; +import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; + +class RelatedChanges extends Composite { + interface Binder extends UiBinder {} + private static Binder uiBinder = GWT.create(Binder.class); + + private static final String OPEN; + private static final HyperlinkImpl link = GWT.create(HyperlinkImpl.class); + + static { + OPEN = DOM.createUniqueId().replace('-', '_'); + init(OPEN); + } + + private static final native void init(String o) /*-{ + $wnd[o] = $entry(function(e,i) { + return @com.google.gerrit.client.change.RelatedChanges::onOpen(Lcom/google/gwt/dom/client/NativeEvent;I)(e,i); + }); + }-*/; + + private static boolean onOpen(NativeEvent e, int idx) { + if (link.handleAsClick(e. cast())) { + MyTable t = getMyTable(e); + if (t != null) { + t.onOpenRow(idx); + e.preventDefault(); + return false; + } + } + return true; + } + + private static MyTable getMyTable(NativeEvent event) { + com.google.gwt.user.client.Element e = event.getEventTarget().cast(); + for (e = DOM.getParent(e); e != null; e = DOM.getParent(e)) { + EventListener l = DOM.getEventListener(e); + if (l instanceof MyTable) { + return (MyTable) l; + } + } + return null; + } + + interface Style extends CssResource { + String subject(); + } + + private String project; + private MyTable table; + private boolean register; + + @UiField Style style; + @UiField Element header; + @UiField ScrollPanel scroll; + @UiField ProgressBar progress; + @UiField Element error; + + RelatedChanges() { + initWidget(uiBinder.createAndBindUi(this)); + } + + void set(ChangeInfo info, final String revision) { + if (info.status().isClosed()) { + setVisible(false); + return; + } + + project = info.project(); + + ChangeApi.revision(info.legacy_id().get(), revision) + .view("related") + .get(new AsyncCallback() { + @Override + public void onSuccess(RelatedInfo result) { + render(revision, result.changes()); + } + + @Override + public void onFailure(Throwable err) { + progress.setVisible(false); + scroll.setVisible(false); + UIObject.setVisible(error, true); + error.setInnerText(err.getMessage()); + } + }); + } + + void setMaxHeight(int height) { + int h = height - header.getOffsetHeight(); + scroll.setHeight(h + "px"); + } + + void registerKeys() { + register = true; + + if (table != null) { + table.setRegisterKeys(true); + } + } + + private void render(String revision, JsArray graph) { + DisplayCommand cmd = new DisplayCommand(revision, graph); + if (cmd.execute()) { + Scheduler.get().scheduleIncremental(cmd); + } + } + + private void setTable(MyTable t) { + progress.setVisible(false); + + scroll.clear(); + scroll.add(t); + scroll.setVisible(true); + table = t; + + if (register) { + table.setRegisterKeys(true); + } + } + + private String url(ChangeAndCommit c) { + if (c.has_change_number() && c.has_revision_number()) { + PatchSet.Id id = c.patch_set_id(); + return "#" + PageLinks.toChange2( + id.getParentKey(), + String.valueOf(id.get())); + } + + GitwebLink gw = Gerrit.getGitwebLink(); + if (gw != null) { + return gw.toRevision(project, c.commit().commit()); + } + return null; + } + + private class MyTable extends NavigationTable { + private final JsArray list; + + MyTable(JsArray list) { + this.list = list; + table.setWidth(""); + + keysNavigation.setName(Gerrit.C.sectionNavigation()); + keysNavigation.add(new PrevKeyCommand(0, 'K', + Resources.C.previousChange())); + keysNavigation.add(new NextKeyCommand(0, 'J', Resources.C.nextChange())); + keysNavigation.add(new OpenKeyCommand(0, 'O', Resources.C.openChange())); + } + + @Override + protected Object getRowItemKey(ChangeAndCommit item) { + return item.id(); + } + + @Override + protected ChangeAndCommit getRowItem(int row) { + if (0 <= row && row <= list.length()) { + return list.get(row); + } + return null; + } + + @Override + protected void onOpenRow(int row) { + if (0 <= row && row <= list.length()) { + ChangeAndCommit c = list.get(row); + String url = url(c); + if (url != null && url.startsWith("#")) { + Gerrit.display(url.substring(1)); + } else if (url != null) { + Window.Location.assign(url); + } + } + } + + void selectRow(int select) { + movePointerTo(select, true); + } + } + + private final class DisplayCommand implements RepeatingCommand { + private final SafeHtmlBuilder sb = new SafeHtmlBuilder(); + private final MyTable table; + private final String revision; + private final JsArray list; + private boolean attached; + private int row; + private int select; + private double start; + + private DisplayCommand(String revision, JsArray list) { + this.table = new MyTable(list); + this.revision = revision; + this.list = list; + } + + public boolean execute() { + boolean attachedNow = isAttached(); + if (!attached && attachedNow) { + // Remember that we have been attached at least once. If + // later we find we aren't attached we should stop running. + attached = true; + } else if (attached && !attachedNow) { + // If the user navigated away, we aren't in the DOM anymore. + // Don't continue to render. + return false; + } + + start = System.currentTimeMillis(); + while (row < list.length()) { + ChangeAndCommit info = list.get(row); + if (revision.equals(info.commit().commit())) { + select = row; + } + render(sb, row, info); + if ((++row % 10) == 0 && longRunning()) { + updateMeter(); + return true; + } + } + table.resetHtml(sb); + setTable(table); + table.selectRow(select); + return false; + } + + private void render(SafeHtmlBuilder sb, int row, ChangeAndCommit info) { + sb.openTr(); + sb.openTd().setStyleName(FileTable.R.css().pointer()).closeTd(); + + sb.openTd().addStyleName(style.subject()); + String url = url(info); + if (url != null) { + sb.openAnchor().setAttribute("href", url); + if (url.startsWith("#")) { + sb.setAttribute("onclick", OPEN + "(event," + row + ")"); + } + sb.append(info.commit().subject()); + sb.closeAnchor(); + } else { + sb.append(info.commit().subject()); + } + sb.closeTd(); + + sb.closeTr(); + } + + private void updateMeter() { + progress.setValue((100 * row) / list.length()); + } + + private boolean longRunning() { + return System.currentTimeMillis() - start > 200; + } + } + + private static class RelatedInfo extends JavaScriptObject { + final native JsArray changes() /*-{ return this.changes }-*/; + protected RelatedInfo() { + } + } + + private static class ChangeAndCommit extends JavaScriptObject { + final native String id() /*-{ return this.change_id }-*/; + final native CommitInfo commit() /*-{ return this.commit }-*/; + + final Change.Id legacy_id() { + return has_change_number() ? new Change.Id(_change_number()) : null; + } + + final PatchSet.Id patch_set_id() { + return has_change_number() && has_revision_number() + ? new PatchSet.Id(legacy_id(), _revision_number()) + : null; + } + + private final native boolean has_change_number() + /*-{ return this.hasOwnProperty('_change_number') }-*/; + + private final native boolean has_revision_number() + /*-{ return this.hasOwnProperty('_revision_number') }-*/; + + private final native int _change_number() + /*-{ return this._change_number }-*/; + + private final native int _revision_number() + /*-{ return this._revision_number }-*/; + + protected ChangeAndCommit() { + } + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.ui.xml new file mode 100644 index 0000000000..1fbaff87eb --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.ui.xml @@ -0,0 +1,41 @@ + + + + + .subject { + width: 200px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + + +
+
+ + Related Changes +
+
+ + +