opendev/gerrit
Code Issues Proposed changes

Indicate outdated dependencies on the ChangeScreen

Browse Source 
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
This commit is contained in:
Martin Fick
2011-06-22 16:32:53 -06:00
parent bc0916318c
commit 827c735f8b
9 changed files with 77 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.common.data;
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 @@ public class ChangeInfo {
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 @@ public class ChangeInfo {
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 @@ public class ChangeInfo {
starred = s;
}
public PatchSet.Id getPatchSetId() {
return patchSetId;
}
public boolean isLatest() {
return latest;
}
public java.sql.Timestamp getLastUpdatedOn() {
return lastUpdatedOn;
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
View File

@@ -138,6 +138,7 @@ public interface GerritCss extends CssResource {
String negscore();
String noLineLineNumber();
String noborder();
String outdated();
String parentsTable();
String patchBrowserPopup();
String patchBrowserPopupBody();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.java
View File

@@ -27,6 +27,7 @@ public interface ChangeMessages extends Messages {
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);

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
View File

@@ -8,6 +8,7 @@ changesAbandonedInProject = Abandoned Changes In {0}
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}

21
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
View File

@@ -18,6 +18,7 @@ import com.google.gerrit.client.Gerrit;
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.FlowPanel;
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 @@ public class ChangeScreen extends Screen {
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 @@ public class ChangeScreen extends Screen {
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 @@ public class ChangeScreen extends Screen {
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;
outdated++;
} else if (ci.getStatus() != Change.Status.MERGED) {
depsOpen = true;
break;
}
}
}
dependenciesPanel.setOpen(depsOpen);
if (outdated > 0) {
dependenciesPanel.getHeader().add(new InlineLabel(
Util.M.outdatedHeader(outdated)));
}
}
private void addComments(final ChangeDetail detail) {

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
View File

@@ -226,6 +226,10 @@ public class ChangeTable extends NavigationTable<ChangeInfo> {
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 @@ public class ChangeTable extends NavigationTable<ChangeInfo> {
@Override
public void go() {
movePointerTo(id);
movePointerTo(cid);
super.go();
}
}

4
gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
View File

@@ -396,6 +396,10 @@
border-spacing: 0;
}
.changeTable .outdated {
background: red;
}
.changeTable .iconCell {
width: 1px;
padding: 0px;

33
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ChangeLink.java
View File

@@ -28,26 +28,31 @@ public class ChangeLink extends InlineHyperlink {
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 @@ public class ChangeLink extends InlineHyperlink {
}
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);
}
}
}

17
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
View File

@@ -202,11 +202,14 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
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 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
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 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
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 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
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;
}