opendev/gerrit
Code Issues Proposed changes

Use the new ChangeDetailCache in PatchSetDisclosurePanel

Browse Source 
Use the ChangeDetailCache to propagate updates from a
PatchSetDisclosurePanel to the ChangeScreen.  This
decouples a PatchSetDisclosurePanel and PatchSetsBlock from
having any knowledge of a ChangeScreen, making them
embeddable in other components now.

Since it is no longer needed, eliminate the
update(ChangeDetail) method on ChangeScreen, instead the
ChangeDetail should be directly updated in the
ChangeDetailCache.  This will provide the additional
benefit of updating any other potential listeners to that
cache.

Change-Id: Iaa49770f6183a4847b4ec9f826f5223aec49be94
This commit is contained in:
Martin Fick
2011-12-29 18:45:50 -07:00
parent b61aff0702
commit ade9eb3c9f
3 changed files with 24 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -251,7 +251,7 @@ public class ChangeScreen extends Screen
patchesGrid.setWidget(0, 1, patchesList);
add(patchesGrid);
patchSetsBlock = new PatchSetsBlock(this);
patchSetsBlock = new PatchSetsBlock();
add(patchSetsBlock);
comments = new FlowPanel();
@@ -284,10 +284,6 @@ public class ChangeScreen extends Screen
}
}
void update(final ChangeDetail detail) {
detailCache.set(detail);
}
private void display(final ChangeDetail detail) {
displayTitle(detail.getChange().getKey(), detail.getChange().getSubject());

43
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
View File

@@ -70,7 +70,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
private static final int R_DOWNLOAD = 3;
private static final int R_CNT = 4;
private final ChangeScreen changeScreen;
private final ChangeDetailCache detailCache;
private final ChangeDetail changeDetail;
private final PatchSet patchSet;
private final FlowPanel body;
@@ -86,27 +86,12 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
* Creates a closed complex disclosure panel for a patch set.
* The patch set details are loaded when the complex disclosure panel is opened.
*/
PatchSetComplexDisclosurePanel(final ChangeScreen parent, final ChangeDetail detail,
final PatchSet ps) {
this(parent, detail, ps, false);
addOpenHandler(this);
}
/**
* Creates an open complex disclosure panel for a patch set.
*/
PatchSetComplexDisclosurePanel(final ChangeScreen parent, final ChangeDetail detail,
final PatchSetDetail psd) {
this(parent, detail, psd.getPatchSet(), true);
ensureLoaded(psd);
}
private PatchSetComplexDisclosurePanel(final ChangeScreen parent, final ChangeDetail detail,
final PatchSet ps, boolean isOpen) {
public PatchSetComplexDisclosurePanel(final PatchSet ps, boolean isOpen) {
super(Util.M.patchSetHeader(ps.getPatchSetId()), isOpen);
changeScreen = parent;
changeDetail = detail;
detailCache = ChangeCache.get(ps.getId().getParentKey()).getChangeDetailCache();
changeDetail = detailCache.get();
patchSet = ps;
body = new FlowPanel();
setContent(body);
@@ -117,7 +102,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
getHeader().add(revtxt);
if (gw != null) {
final Anchor revlink =
new Anchor(gw.getLinkName(), false, gw.toRevision(detail.getChange()
new Anchor(gw.getLinkName(), false, gw.toRevision(changeDetail.getChange()
.getProject(), ps));
revlink.addStyleName(Gerrit.RESOURCES.css().patchSetLink());
getHeader().add(revlink);
@@ -128,6 +113,12 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
draftLabel.addStyleName(Gerrit.RESOURCES.css().patchSetRevision());
getHeader().add(draftLabel);
}
if (isOpen) {
ensureLoaded(changeDetail.getCurrentPatchSetDetail());
} else {
addOpenHandler(this);
}
}
public void setDiffBaseId(PatchSet.Id diffBaseId) {
@@ -611,7 +602,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
Util.MANAGE_SVC.publish(patchSet.getId(),
new GerritCallback<ChangeDetail>() {
public void onSuccess(ChangeDetail result) {
changeScreen.update(result);
detailCache.set(result);
}
@Override
@@ -635,7 +626,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
new GerritCallback<ChangeDetail>() {
public void onSuccess(final ChangeDetail result) {
if (result != null) {
changeScreen.update(result);
detailCache.set(result);
} else {
Gerrit.display(PageLinks.MINE);
}
@@ -728,7 +719,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
new SubmitFailureDialog(result, msg).center();
}
}
changeScreen.update(result);
detailCache.set(result);
}
public PatchSet getPatchSet() {
@@ -757,13 +748,13 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
private abstract class ActionDialog extends CommentedActionDialog<ChangeDetail> {
public ActionDialog(final FocusWidget enableOnFailure, final boolean redirect,
String dialogTitle, String dialogHeading) {
super(dialogTitle, dialogHeading, new AsyncCallback<ChangeDetail>() {
super(dialogTitle, dialogHeading, new ChangeDetailCache.IgnoreErrorCallback() {
@Override
public void onSuccess(ChangeDetail result) {
if (redirect) {
Gerrit.display(PageLinks.toChange(result.getChange().getId()));
} else {
changeScreen.update(result);
super.onSuccess(result);
}
}

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

@@ -43,11 +43,9 @@ import java.util.Map;
* that keyboard navigation to each changed file in all patch sets is possible.
*/
public class PatchSetsBlock extends Composite {
private final Map<PatchSet.Id, PatchSetComplexDisclosurePanel> patchSetPanels =
new HashMap<PatchSet.Id, PatchSetComplexDisclosurePanel>();
private final ChangeScreen parent;
private final FlowPanel body;
private HandlerRegistration regNavigation;
@@ -65,8 +63,7 @@ public class PatchSetsBlock extends Composite {
/** Patch sets on this change, in order. */
private List<PatchSet> patchSets;
PatchSetsBlock(final ChangeScreen parent) {
this.parent = parent;
PatchSetsBlock() {
body = new FlowPanel();
initWidget(body);
}
@@ -90,19 +87,13 @@ public class PatchSetsBlock extends Composite {
patchSetPanelsList = new ArrayList<PatchSetComplexDisclosurePanel>();
for (final PatchSet ps : patchSets) {
final PatchSetComplexDisclosurePanel p;
if (ps == currps) {
p = new PatchSetComplexDisclosurePanel(parent, detail, detail
.getCurrentPatchSetDetail());
if (diffBaseId != null) {
p.setDiffBaseId(diffBaseId);
final PatchSetComplexDisclosurePanel p =
new PatchSetComplexDisclosurePanel(ps, ps == currps);
if (diffBaseId != null) {
p.setDiffBaseId(diffBaseId);
if (ps == currps) {
p.refresh();
}
} else {
p = new PatchSetComplexDisclosurePanel(parent, detail, ps);
if (diffBaseId != null) {
p.setDiffBaseId(diffBaseId);
}
}
add(p);
patchSetPanelsList.add(p);