From ade9eb3c9f8fcbf7b53486a188cfc0e6870c8c71 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Thu, 29 Dec 2011 18:45:50 -0700 Subject: [PATCH] Use the new ChangeDetailCache in PatchSetDisclosurePanel 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 --- .../gerrit/client/changes/ChangeScreen.java | 6 +-- .../PatchSetComplexDisclosurePanel.java | 43 ++++++++----------- .../gerrit/client/changes/PatchSetsBlock.java | 21 +++------ 3 files changed, 24 insertions(+), 46 deletions(-) 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 6806b13cf7..3b820b3ad2 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 @@ -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()); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java index c2fad8c002..199dc9d808 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java @@ -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() { public void onSuccess(ChangeDetail result) { - changeScreen.update(result); + detailCache.set(result); } @Override @@ -635,7 +626,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel new GerritCallback() { 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 { public ActionDialog(final FocusWidget enableOnFailure, final boolean redirect, String dialogTitle, String dialogHeading) { - super(dialogTitle, dialogHeading, new AsyncCallback() { + 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); } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetsBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetsBlock.java index 8764bbaf7c..7e659a1b46 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetsBlock.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetsBlock.java @@ -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 patchSetPanels = new HashMap(); - 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 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(); 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);