Enhance UpToChange link to activate the last browsed patchset.

While returning to the Change using "UpToChange" link in any
patchset, always the latest patchset is activated. This commit
modifies the functionality of the "UpToChange" link by storing
the last browsed patchset and activating it on return. This
makes it easy to review older patchsets.

Bug: issue 822
Change-Id: Iaf01c53797d92434a0bc150f73ad232aec3619d6

gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java

@@ -18,6 +18,7 @@ import com.google.gerrit.common.data.AccountInfo;
 import com.google.gerrit.common.data.ChangeInfo;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gerrit.reviewdb.Project;
 import com.google.gerrit.reviewdb.Change.Status;
 import com.google.gwtorm.client.KeyUtil;
@@ -49,6 +50,10 @@ public class PageLinks {
     return "change," + c.toString();
   }
 
+  public static String toChange(final PatchSet.Id ps) {
+    return "change," + ps.getParentKey().toString() + ",patchset=" + ps.get();
+  }
+
   public static String toAccountDashboard(final AccountInfo acct) {
     return toAccountDashboard(acct.getId());
   }

gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java

@@ -232,8 +232,15 @@ public class Dispatcher {
     String p;
 
     p = "change,";
-    if (token.startsWith(p))
+    if (token.startsWith(p)) {
-      return new ChangeScreen(Change.Id.parse(skip(p, token)));
+      final String s = skip(p, token);
+      final String q = "patchset=";
+      final String t[] = s.split(",", 2);
+      if (t.length > 1 && t[1].startsWith(q)) {
+        return new ChangeScreen(PatchSet.Id.parse(t[0] + "," + skip(q, t[1])));
+      }
+      return new ChangeScreen(Change.Id.parse(t[0]));
+    }
 
     p = "dashboard,";
     if (token.startsWith(p))

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

@@ -54,6 +54,7 @@ import java.util.List;
 
 public class ChangeScreen extends Screen {
   private final Change.Id changeId;
+  private final PatchSet.Id openPatchSetId;
 
   private Image starChange;
   private boolean starred;
@@ -78,6 +79,12 @@ public class ChangeScreen extends Screen {
 
   public ChangeScreen(final Change.Id toShow) {
     changeId = toShow;
+    openPatchSetId = null;
+  }
+
+  public ChangeScreen(final PatchSet.Id toShow) {
+    changeId = toShow.getParentKey();
+    openPatchSetId = toShow;
   }
 
   public ChangeScreen(final ChangeInfo c) {
@@ -251,6 +258,9 @@ public class ChangeScreen extends Screen {
         .getApprovals());
 
     patchSetsBlock.display(detail);
+    if (openPatchSetId != null) {
+      patchSetsBlock.activate(openPatchSetId);
+    }
     addComments(detail);
 
     // If any dependency change is still open, show our dependency list.

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.client.changes;
 
 import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.common.data.ChangeDetail;
 import com.google.gerrit.reviewdb.AccountGeneralPreferences;
 import com.google.gerrit.reviewdb.PatchSet;
@@ -151,14 +152,18 @@ public class PatchSetsBlock extends Composite {
    * This method also ensures that the current row is only highlighted in the
    * table of the active patch set panel.
    */
-  private void activate(final PatchSet.Id patchSetId) {
+  public void activate(final PatchSet.Id patchSetId) {
-    if (!patchSetId.equals(activePatchSetId)) {
+    if (indexOf(patchSetId) != -1) {
-      deactivate();
+      if (!patchSetId.equals(activePatchSetId)) {
-      PatchSetComplexDisclosurePanel patchSetPanel =
+        deactivate();
-          patchSetPanels.get(patchSetId);
+        PatchSetComplexDisclosurePanel patchSetPanel =
-      patchSetPanel.setOpen(true);
+            patchSetPanels.get(patchSetId);
-      patchSetPanel.setActive(true);
+        patchSetPanel.setOpen(true);
-      activePatchSetId = patchSetId;
+        patchSetPanel.setActive(true);
+        activePatchSetId = patchSetId;
+      }
+    } else {
+      Gerrit.display(PageLinks.toChange(patchSetId.getParentKey()));
     }
   }
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/NavLinks.java

@@ -19,7 +19,7 @@ import com.google.gerrit.client.changes.PatchTable;
 import com.google.gerrit.client.changes.Util;
 import com.google.gerrit.client.ui.ChangeLink;
 import com.google.gerrit.client.ui.InlineHyperlink;
-import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gwt.event.dom.client.KeyPressEvent;
 import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.Grid;
@@ -47,14 +47,14 @@ class NavLinks extends Composite {
     }
   }
 
-  private final Change.Id changeId;
+  private final PatchSet.Id patchSetId;
   private final KeyCommandSet keys;
   private final Grid table;
 
   private KeyCommand cmds[] = new KeyCommand[2];
 
-  NavLinks(KeyCommandSet kcs, Change.Id forChange) {
+  NavLinks(KeyCommandSet kcs, PatchSet.Id forPatch) {
-    changeId = forChange;
+    patchSetId = forPatch;
     keys = kcs;
     table = new Grid(1, 3);
     initWidget(table);
@@ -65,7 +65,7 @@ class NavLinks extends Composite {
     fmt.setHorizontalAlignment(0, 1, HasHorizontalAlignment.ALIGN_CENTER);
     fmt.setHorizontalAlignment(0, 2, HasHorizontalAlignment.ALIGN_RIGHT);
 
-    final ChangeLink up = new ChangeLink("", changeId);
+    final ChangeLink up = new ChangeLink("", patchSetId);
     SafeHtml.set(up, SafeHtml.asis(Util.C.upToChangeIconLink()));
     table.setWidget(0, 1, up);
   }
@@ -104,7 +104,7 @@ class NavLinks extends Composite {
           }
         };
       } else {
-        cmds[nav.cmd] = new UpToChangeCommand(changeId, 0, nav.key);
+        cmds[nav.cmd] = new UpToChangeCommand(patchSetId, 0, nav.key);
       }
 
       keys.add(cmds[nav.cmd]);

gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java

@@ -250,9 +250,8 @@ public abstract class PatchScreen extends Screen implements
   protected void onInitUI() {
     super.onInitUI();
 
-    final Change.Id ck = patchKey.getParentKey().getParentKey();
     keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
-    keysNavigation.add(new UpToChangeCommand(ck, 0, 'u'));
+    keysNavigation.add(new UpToChangeCommand(patchKey.getParentKey(), 0, 'u'));
     keysNavigation.add(new FileListCmd(0, 'f', PatchUtil.C.fileList()));
 
     historyTable = new HistoryTable(this);
@@ -284,9 +283,8 @@ public abstract class PatchScreen extends Screen implements
     contentTable = createContentTable();
     contentTable.fileList = fileList;
 
-    topNav =
+    topNav = new NavLinks(keysNavigation, patchKey.getParentKey());
-        new NavLinks(keysNavigation, patchKey.getParentKey().getParentKey());
+    bottomNav = new NavLinks(null, patchKey.getParentKey());
-    bottomNav = new NavLinks(null, patchKey.getParentKey().getParentKey());
 
     add(topNav);
     contentPanel = new FlowPanel();

gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/UpToChangeCommand.java

@@ -17,20 +17,20 @@ package com.google.gerrit.client.patches;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.changes.ChangeScreen;
 import com.google.gerrit.common.PageLinks;
-import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gwt.event.dom.client.KeyPressEvent;
 import com.google.gwtexpui.globalkey.client.KeyCommand;
 
 class UpToChangeCommand extends KeyCommand {
-  private final Change.Id changeId;
+  private final PatchSet.Id patchSetId;
 
-  UpToChangeCommand(Change.Id changeId, int mask, int key) {
+  UpToChangeCommand(PatchSet.Id patchSetId, int mask, int key) {
     super(mask, key, PatchUtil.C.upToChange());
-    this.changeId = changeId;
+    this.patchSetId = patchSetId;
   }
 
   @Override
   public void onKeyPress(final KeyPressEvent event) {
-    Gerrit.display(PageLinks.toChange(changeId), new ChangeScreen(changeId));
+    Gerrit.display(PageLinks.toChange(patchSetId), new ChangeScreen(patchSetId));
   }
 }

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

@@ -19,6 +19,7 @@ import com.google.gerrit.client.changes.ChangeScreen;
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.common.data.ChangeInfo;
 import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.user.client.DOM;
 
@@ -28,12 +29,20 @@ public class ChangeLink extends InlineHyperlink {
   }
 
   protected Change.Id id;
+  protected PatchSet.Id ps;
   private ChangeInfo info;
 
   public ChangeLink(final String text, final Change.Id c) {
     super(text, PageLinks.toChange(c));
     DOM.setElementProperty(getElement(), "href", permalink(c));
     id = c;
+    ps = null;
+  }
+
+  public ChangeLink(final String text, final PatchSet.Id ps) {
+    super(text, PageLinks.toChange(ps));
+    id = ps.getParentKey();
+    this.ps = ps;
   }
 
   public ChangeLink(final String text, final ChangeInfo c) {
@@ -47,6 +56,12 @@ public class ChangeLink extends InlineHyperlink {
   }
 
   private Screen createScreen() {
-    return info != null ? new ChangeScreen(info) : new ChangeScreen(id);
+    if (info != null) {
+      return new ChangeScreen(info);
+    } else if (ps != null) {
+      return new ChangeScreen(ps);
+    } else {
+      return new ChangeScreen(id);
+    }
   }
 }