Indicate outdated dependencies on the ChangeScreen
If change dependency no longer the latest patchSet for that
change, mark it OUTDATED in the dependencies table and make
its row red, and add a warning message to the dependencies
header. Additionally make the link for dependencies link to
the exact patchSet of the dependent change. Lastly, keep the
dependencies disclosure panel open even when an outdated
dependent change is merged.
Bug issue: 855
Change-Id: I238431eeccba3c41555ef229e4003c3910746499
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java
index b5271ec..eac8a9e 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java
@@ -16,6 +16,7 @@
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.PatchSet;
import java.sql.Timestamp;
@@ -31,11 +32,13 @@
protected boolean starred;
protected Timestamp lastUpdatedOn;
protected String sortKey;
+ protected PatchSet.Id patchSetId;
+ protected boolean latest;
protected ChangeInfo() {
}
- public ChangeInfo(final Change c) {
+ public ChangeInfo(final Change c, final PatchSet.Id patchId) {
id = c.getId();
key = c.getKey();
owner = c.getOwner();
@@ -46,6 +49,12 @@
topic = c.getTopic();
lastUpdatedOn = c.getLastUpdatedOn();
sortKey = c.getSortKey();
+ patchSetId = patchId;
+ latest = patchSetId == null || c.currPatchSetId().equals(patchSetId);
+ }
+
+ public ChangeInfo(final Change c) {
+ this(c, null);
}
public Change.Id getId() {
@@ -88,6 +97,14 @@
starred = s;
}
+ public PatchSet.Id getPatchSetId() {
+ return patchSetId;
+ }
+
+ public boolean isLatest() {
+ return latest;
+ }
+
public java.sql.Timestamp getLastUpdatedOn() {
return lastUpdatedOn;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
index 993d06a..244f887 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
@@ -138,6 +138,7 @@
String negscore();
String noLineLineNumber();
String noborder();
+ String outdated();
String parentsTable();
String patchBrowserPopup();
String patchBrowserPopupBody();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
index 53d1c18..728d567 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
@@ -27,6 +27,7 @@
String revertChangeDefaultMessage(String commitMsg, String commitId);
String changeScreenTitleId(String changeId);
+ String outdatedHeader(int outdated);
String patchSetHeader(int id);
String loadingPatchSet(int id);
String submitPatchSet(int id);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
index 6f6e34e2..3b908f3 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
@@ -8,6 +8,7 @@
revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}
changeScreenTitleId = Change {0}
+outdatedHeader = Change depends on {0} outdated change(s) and should be rebased on the latest patch sets.
patchSetHeader = Patch Set {0}
loadingPatchSet = Loading Patch Set {0} ...
submitPatchSet = Submit Patch Set {0}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
index 6bd4ebf..6da0ceab 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
@@ -18,6 +18,7 @@
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.client.ui.CommentPanel;
+import com.google.gerrit.client.ui.ComplexDisclosurePanel;
import com.google.gerrit.client.ui.ExpandAllCommand;
import com.google.gerrit.client.ui.LinkMenuBar;
import com.google.gerrit.client.ui.NeedsSignInKeyCommand;
@@ -44,6 +45,7 @@
import com.google.gwt.user.client.ui.Grid;
import com.google.gwt.user.client.ui.HorizontalPanel;
import com.google.gwt.user.client.ui.Image;
+import com.google.gwt.user.client.ui.InlineLabel;
import com.google.gwt.user.client.ui.Label;
import com.google.gwt.user.client.ui.ListBox;
import com.google.gwt.user.client.ui.Panel;
@@ -67,7 +69,7 @@
private IncludedInTable includedInTable;
private DisclosurePanel includedInPanel;
- private DisclosurePanel dependenciesPanel;
+ private ComplexDisclosurePanel dependenciesPanel;
private ChangeTable dependencies;
private ChangeTable.Section dependsOn;
private ChangeTable.Section neededBy;
@@ -226,7 +228,8 @@
dependencies.addSection(dependsOn);
dependencies.addSection(neededBy);
- dependenciesPanel = new DisclosurePanel(Util.C.changeScreenDependencies());
+ dependenciesPanel = new ComplexDisclosurePanel(
+ Util.C.changeScreenDependencies(), false);
dependenciesPanel.setContent(dependencies);
add(dependenciesPanel);
@@ -322,20 +325,28 @@
patchSetsBlock.display(detail, diffBaseId);
addComments(detail);
- // If any dependency change is still open, show our dependency list.
+ // If any dependency change is still open, or is outdated,
+ // show our dependency list.
//
boolean depsOpen = false;
+ int outdated = 0;
if (!detail.getChange().getStatus().isClosed()
&& detail.getDependsOn() != null) {
for (final ChangeInfo ci : detail.getDependsOn()) {
- if (ci.getStatus() != Change.Status.MERGED) {
+ if (! ci.isLatest()) {
depsOpen = true;
- break;
+ outdated++;
+ } else if (ci.getStatus() != Change.Status.MERGED) {
+ depsOpen = true;
}
}
}
dependenciesPanel.setOpen(depsOpen);
+ if (outdated > 0) {
+ dependenciesPanel.getHeader().add(new InlineLabel(
+ Util.M.outdatedHeader(outdated)));
+ }
}
private void addComments(final ChangeDetail detail) {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
index eb7014e..3b71ee2 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
@@ -226,6 +226,10 @@
if (c.getStatus() != null && c.getStatus() != Change.Status.NEW) {
s += " (" + c.getStatus().name() + ")";
}
+ if (! c.isLatest()) {
+ s += " [OUTDATED]";
+ table.getRowFormatter().addStyleName(row, Gerrit.RESOURCES.css().outdated());
+ }
table.setWidget(row, C_SUBJECT, new TableChangeLink(s, c));
table.setWidget(row, C_OWNER, link(c.getOwner()));
table.setWidget(row, C_PROJECT, new ProjectLink(c.getProject().getKey(), c
@@ -426,7 +430,7 @@
@Override
public void go() {
- movePointerTo(id);
+ movePointerTo(cid);
super.go();
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
index a281393..e8fbc48 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
@@ -396,6 +396,10 @@
border-spacing: 0;
}
+.changeTable .outdated {
+ background: red;
+}
+
.changeTable .iconCell {
width: 1px;
padding: 0px;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ChangeLink.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ChangeLink.java
index 0d0dfdb..f022f7e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ChangeLink.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ChangeLink.java
@@ -28,26 +28,31 @@
return GWT.getHostPageBaseURL() + c.get();
}
- protected Change.Id id;
- protected PatchSet.Id ps;
- private ChangeInfo info;
+ protected Change.Id cid;
+ protected PatchSet.Id psid;
public ChangeLink(final String text, final Change.Id c) {
super(text, PageLinks.toChange(c));
DOM.setElementProperty(getElement(), "href", permalink(c));
- id = c;
- ps = null;
+ cid = c;
+ psid = null;
}
public ChangeLink(final String text, final PatchSet.Id ps) {
super(text, PageLinks.toChange(ps));
- id = ps.getParentKey();
- this.ps = ps;
+ cid = ps.getParentKey();
+ psid = ps;
}
- public ChangeLink(final String text, final ChangeInfo c) {
- this(text, c.getId());
- info = c;
+ public ChangeLink(final String text, final ChangeInfo info) {
+ super(text, getTarget(info));
+ cid = info.getId();
+ psid = info.getPatchSetId();
+ }
+
+ public static String getTarget(final ChangeInfo info) {
+ PatchSet.Id ps = info.getPatchSetId();
+ return (ps == null) ? PageLinks.toChange(info) : PageLinks.toChange(ps);
}
@Override
@@ -56,12 +61,10 @@
}
private Screen createScreen() {
- if (info != null) {
- return new ChangeScreen(info);
- } else if (ps != null) {
- return new ChangeScreen(ps);
+ if (psid != null) {
+ return new ChangeScreen(psid);
} else {
- return new ChangeScreen(id);
+ return new ChangeScreen(cid);
}
}
}
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 2705991..c274702 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
@@ -202,11 +202,14 @@
detail.setCurrentPatchSetDetail(loader.call());
final HashSet<Change.Id> changesToGet = new HashSet<Change.Id>();
+ final HashMap<Change.Id,PatchSet.Id> ancestorPatchIds =
+ new HashMap<Change.Id,PatchSet.Id>();
final List<Change.Id> ancestorOrder = new ArrayList<Change.Id>();
for (PatchSetAncestor a : db.patchSetAncestors().ancestorsOf(psId)) {
for (PatchSet p : db.patchSets().byRevision(a.getAncestorRevision())) {
final Change.Id ck = p.getId().getParentKey();
if (changesToGet.add(ck)) {
+ ancestorPatchIds.put(ck, p.getId());
ancestorOrder.add(ck);
}
}
@@ -229,7 +232,7 @@
for (final Change.Id a : ancestorOrder) {
final Change ac = m.get(a);
if (ac != null) {
- dependsOn.add(newChangeInfo(ac));
+ dependsOn.add(newChangeInfo(ac, ancestorPatchIds));
}
}
@@ -237,7 +240,7 @@
for (final Change.Id a : descendants) {
final Change ac = m.get(a);
if (ac != null) {
- neededBy.add(newChangeInfo(ac));
+ neededBy.add(newChangeInfo(ac, null));
}
}
@@ -251,9 +254,15 @@
detail.setNeededBy(neededBy);
}
- private ChangeInfo newChangeInfo(final Change ac) {
+ private ChangeInfo newChangeInfo(final Change ac,
+ Map<Change.Id,PatchSet.Id> ancestorPatchIds) {
aic.want(ac.getOwner());
- ChangeInfo ci = new ChangeInfo(ac);
+ ChangeInfo ci;
+ if (ancestorPatchIds == null) {
+ ci = new ChangeInfo(ac);
+ } else {
+ ci = new ChangeInfo(ac, ancestorPatchIds.get(ac.getId()));
+ }
ci.setStarred(isStarred(ac));
return ci;
}
