[Bug]Fix pagination when ApiOptions.StartPage is explicitly set.

octokit · Sep 22, 2023 · 57a29e9 · 57a29e9
1 parent dcc31b8
commit 57a29e9
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 1 deletion.
diff --git a/Octokit.Tests/Helpers/UriExtensionsTests.cs b/Octokit.Tests/Helpers/UriExtensionsTests.cs
@@ -161,6 +161,64 @@ public void EnsuresArgumentNotNull()
                 Uri uri = null;
                 Assert.Throws<ArgumentNullException>(() => uri.ApplyParameters(new Dictionary<string, string>()));
             }
+
+            [Fact]
+            public void AppendsStartPageIfNone()
+            {
+                var uri = new Uri("https://api.github.com/repositories/1/labels");
+
+                var parameters = new Dictionary<string, string>();
+                Pagination.Setup(parameters, new ApiOptions { StartPage = 3, PageSize = 7 });
+
+                var actual = uri.ApplyParameters(parameters);
+
+                Assert.Equal(
+                    new Uri("https://api.github.com/repositories/1/labels?per_page=7&page=3"),
+                    actual);
+            }
+
+            [Fact]
+            public void AppendsStartPageIfNoneForRelativeUri()
+            {
+                var uri = new Uri("/repositories/1/labels", UriKind.Relative);
+
+                var parameters = new Dictionary<string, string>();
+                Pagination.Setup(parameters, new ApiOptions { StartPage = 3, PageSize = 7 });
+
+                var actual = uri.ApplyParameters(parameters);
+
+                Assert.Equal(
+                    new Uri("/repositories/1/labels?per_page=7&page=3", UriKind.Relative),
+                    actual);
+            }
+
+            [Fact]
+            public void DoesNotChangePageNumberInNextPageUriWithStartPage()
+            {
+                const string nextPageUriString = "https://api.github.com/repositories/1/labels?per_page=5&page=2";
+                var nextPageUri = new Uri(nextPageUriString);
+
+                var parameters = new Dictionary<string, string>();
+                Pagination.Setup(parameters, new ApiOptions { StartPage = 1, PageSize = 5 });
+
+                var actual = nextPageUri.ApplyParameters(parameters);
+
+                Assert.Equal(new Uri(nextPageUriString), actual);
+            }
+
+            [Fact]
+            public void DoesNotChangePageNumberInNextPageUriWithStartPageForRelativeUri()
+            {
+                const string nextPageUriString = "/repositories/1/labels?per_page=5&page=2";
+                var nextPageUri = new Uri(nextPageUriString, UriKind.Relative);
+
+                var parameters = new Dictionary<string, string>();
+                Pagination.Setup(parameters, new ApiOptions { StartPage = 1, PageSize = 5 });
+
+                var actual = nextPageUri.ApplyParameters(parameters);
+
+                Assert.Equal(new Uri(nextPageUriString, UriKind.Relative), actual);
+            }
         }
     }
 }
diff --git a/Octokit/Helpers/UriExtensions.cs b/Octokit/Helpers/UriExtensions.cs
@@ -76,7 +76,14 @@ public static Uri ApplyParameters(this Uri uri, IDictionary<string, string> para
 
             foreach (var existing in existingParameters)
             {
-                if (!p.ContainsKey(existing.Key))
+                if (existing.Key == "page")
+                {
+                    // See https://github.com/octokit/octokit.net/issues/1955
+                    // See https://github.com/octokit/octokit.net/issues/2602
+                    // Desired behavior: don't replace page number in nextPageUri
+                    p[existing.Key] = existing.Value;
+                }
+                else if (!p.ContainsKey(existing.Key))
                 {
                     p.Add(existing);
                 }