commit e4ae3599cb960c21bf490b462e0a68e7e5f0adaa
author Sam Delmerico <delmerico@google.com> Wed Jan 31 14:44:58 2024 -0800
committer Sam Delmerico <delmerico@google.com> Thu Feb 01 11:10:24 2024 -0800
tree 942706df71c6c6eb8113c2501c917fa6f8a5c31e
parent 132683bf56881c674d630e5524c7b5260228a7d1

ensure that s= param is set correctly for previous link in log

On the second page of results in a log view, the Previous button links back
to itself rather than to the first page of results because the start param,
s=, was not properly removed.

Change-Id: If6102b727740438e45f549bdda053b7089955001

java/com/google/gitiles/GitilesView.java

diff --git a/java/com/google/gitiles/GitilesView.java b/java/com/google/gitiles/GitilesView.java
index 9bb43a9..f5be973 100644
--- a/java/com/google/gitiles/GitilesView.java
+++ b/java/com/google/gitiles/GitilesView.java
@@ -32,6 +32,7 @@
 import com.google.common.collect.LinkedListMultimap;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Multimaps;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
 import java.util.ArrayList;
@@ -299,6 +300,12 @@
       return this;
     }
 
+    @CanIgnoreReturnValue
+    public Builder removeParam(String key) {
+      params.removeAll(key);
+      return this;
+    }
+
     public Builder putAllParams(Map<String, String[]> params) {
       for (Map.Entry<String, String[]> e : params.entrySet()) {
         this.params.putAll(e.getKey(), Arrays.asList(e.getValue()));

java/com/google/gitiles/LogSoyData.java

diff --git a/java/com/google/gitiles/LogSoyData.java b/java/com/google/gitiles/LogSoyData.java
index 0c57e69..4292bc9 100644
--- a/java/com/google/gitiles/LogSoyData.java
+++ b/java/com/google/gitiles/LogSoyData.java
@@ -71,7 +71,10 @@
     this.view = checkNotNull(ViewFilter.getView(req));
     checkNotNull(pretty);
     Config config = access.getConfig();
-    fields = config.getBoolean("logFormat", pretty, "verbose", false) || pretty.equals("fuller") ? VERBOSE_FIELDS : FIELDS;
+    fields =
+        config.getBoolean("logFormat", pretty, "verbose", false) || pretty.equals("fuller")
+            ? VERBOSE_FIELDS
+            : FIELDS;
     variant = firstNonNull(config.getString("logFormat", pretty, "variant"), pretty);
   }
 
@@ -120,7 +123,9 @@
     ObjectId prev = paginator.getPreviousStart();
     if (prev != null) {
       GitilesView.Builder prevView = copyAndCanonicalizeView(revision);
-      if (!prevView.getRevision().getId().equals(prev)) {
+      if (prevView.getRevision().getId().equals(prev)) {
+        prevView.removeParam(LogServlet.START_PARAM);
+      } else {
         prevView.replaceParam(LogServlet.START_PARAM, prev.name());
       }
       data.put("previousUrl", prevView.toUrl());

javatests/com/google/gitiles/LogServletTest.java

diff --git a/javatests/com/google/gitiles/LogServletTest.java b/javatests/com/google/gitiles/LogServletTest.java
index 4ef7a4d..94c2e38 100644
--- a/javatests/com/google/gitiles/LogServletTest.java
+++ b/javatests/com/google/gitiles/LogServletTest.java
@@ -15,6 +15,7 @@
 package com.google.gitiles;
 
 import static com.google.common.truth.Truth.assertThat;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.gitiles.CommitJsonData.Commit;
 import com.google.gitiles.CommitJsonData.Log;
@@ -30,6 +31,7 @@
 @RunWith(JUnit4.class)
 public class LogServletTest extends ServletTest {
   private static final TypeToken<Log> LOG = new TypeToken<Log>() {};
+  private static final String MAIN = "main";
 
   @Test
   public void basicLog() throws Exception {
@@ -136,4 +138,33 @@
     assertThat(jsonCommit.committer.time).isEqualTo(df.format(commit.getCommitterIdent()));
     assertThat(jsonCommit.message).isEqualTo(commit.getFullMessage());
   }
+
+  @Test
+  public void verifyPreviousButtonAction() throws Exception {
+    repo.branch(MAIN).commit().add("foo", "contents").create();
+    RevCommit grandParent = repo.branch(MAIN).commit().add("foo", "contents").create();
+    RevCommit parent =
+        repo.branch(MAIN).commit().parent(grandParent).add("foo", "contents").create();
+    RevCommit main = repo.branch(MAIN).commit().parent(parent).create();
+
+    int numCommitsPerPage = 2;
+    String path =
+        "/repo/+log/" + grandParent.toObjectId().getName() + ".." + main.toObjectId().getName();
+    FakeHttpServletResponse res =
+        buildResponse(
+            path,
+            "format=html" + "&n=" + numCommitsPerPage + "&s=" + parent.toObjectId().getName(),
+            SC_OK);
+
+    assertThat(res.getActualBodyString())
+        .contains(
+            "<a class=\"LogNav-prev\""
+                + " href=\"/b/repo/+log/"
+                + grandParent.toObjectId().getName()
+                + ".."
+                + main.toObjectId().getName()
+                + "/?format=html"
+                + "&amp;n=2"
+                + "\">");
+  }
 }