Fix #2693 PageCount doesn't work if the query gets constructed with p…

…age number as the first query parameter (#2911) Fix #2693 Co-authored-by: Keegan Campbell <[email protected]>
octokit · Apr 30, 2024 · a317340 · a317340
1 parent 110b4e3
commit a317340
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/Octokit.Tests/Http/PaginationTests.cs b/Octokit.Tests/Http/PaginationTests.cs
@@ -0,0 +1,46 @@
+using System;
+using Octokit.Models.Request.Enterprise;
+using Xunit;
+
+namespace Octokit.Tests.Http
+{
+    public class PaginationTests
+    {
+        public class TheShouldContinueMethod
+        {
+            [Fact]
+            public void HandlesMissingUri()
+            {
+                var result = Pagination.ShouldContinue(null, null);
+                Assert.False(result);
+            }
+
+            [Fact]
+            public void HandlesIsDone()
+            {
+                var uri = new Uri("http://example.com");
+                var options = new ApiOptionsExtended { IsDone = true };
+                var result = Pagination.ShouldContinue(uri, options);
+                Assert.False(result);
+            }
+
+            [Fact]
+            public void HandlesPageCountPageFirstParam()
+            {
+                var uri = new Uri("http://example.com?page=2");
+                var options = new ApiOptions { StartPage = 1, PageCount = 1 };
+                var result = Pagination.ShouldContinue(uri, options);
+                Assert.False(result);
+            }
+
+            [Fact]
+            public void HandlesPageCountPageNotFirstParam()
+            {
+                var uri = new Uri("http://example.com?page_size=100&page=2");
+                var options = new ApiOptions { StartPage = 1, PageCount = 1 };
+                var result = Pagination.ShouldContinue(uri, options);
+                Assert.False(result);
+            }
+        }
+    }
+}
diff --git a/Octokit/Helpers/Pagination.cs b/Octokit/Helpers/Pagination.cs
@@ -41,8 +41,7 @@ internal static bool ShouldContinue(
             {
                 var allValues = ToQueryStringDictionary(uri);
 
-                string pageValue;
-                if (allValues.TryGetValue("page", out pageValue))
+                if (allValues.TryGetValue("page", out var pageValue))
                 {
                     var startPage = options.StartPage ?? 1;
                     var pageCount = options.PageCount.Value;
@@ -61,8 +60,14 @@ internal static bool ShouldContinue(
         static Dictionary<string, string> ToQueryStringDictionary(Uri uri)
         {
             return uri.Query.Split('&')
-                .Select(keyValue =>
+                .Select((keyValue, i) =>
                 {
+                    if (i == 0)
+                    {
+                        // Trim the leading '?' character from the first key-value pair
+                        keyValue = keyValue.Substring(1);
+                    }
+
                     var indexOf = keyValue.IndexOf('=');
                     if (indexOf > 0)
                     {