Merge pull request #4780 from rmosolgo/fix-has-next-page-with-max-page-size

Fix hasNextPage when max_page_size is set

Author: rmosolgo

lib/graphql/pagination/array_connection.rb

@@ -35,10 +35,10 @@ def index_from_cursor(cursor)
       def load_nodes
         @nodes ||= begin
           sliced_nodes = if before && after
-            end_idx = index_from_cursor(before)-1
+            end_idx = index_from_cursor(before) - 2
             end_idx < 0 ? [] : items[index_from_cursor(after)..end_idx] || []
           elsif before
-            end_idx = index_from_cursor(before)-2
+            end_idx = index_from_cursor(before) - 2
             end_idx < 0 ? [] : items[0..end_idx] || []
           elsif after
             items[index_from_cursor(after)..-1] || []
@@ -56,7 +56,7 @@ def load_nodes
             false
           end
 
-          @has_next_page = if first
+          @has_next_page = if first_value && first
             # There are more items after these items
             sliced_nodes.count > first
           elsif before

lib/graphql/pagination/relation_connection.rb

@@ -29,14 +29,14 @@ def has_previous_page
 
       def has_next_page
         if @has_next_page.nil?
-          @has_next_page = if before_offset && before_offset > 0
-            true
-          elsif first
+          @has_next_page = if first && first_value
             if @nodes && @nodes.count < first
               false
             else
               relation_larger_than(sliced_nodes, @sliced_nodes_offset, first)
             end
+          elsif before_offset && before_offset > 0
+            true
           else
             false
           end

spec/integration/mongoid/graphql/relay/mongo_relation_connection_spec.rb

@@ -238,7 +238,7 @@ def get_last_cursor(result)
         # Max page size is applied _without_ `first`, also
         result = star_trek_query(query_string)
         assert_equal(2, result["data"]["federation"]["bases"]["edges"].size)
-        assert_equal(true, result["data"]["federation"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is true when first is not specified")
+        assert_equal(false, result["data"]["federation"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is false when first is not specified")
       end
 
       it "applies to queries by `last`" do
@@ -297,7 +297,7 @@ def get_last_cursor(result)
         # Max page size is applied _without_ `first`, also
         result = star_trek_query(query_string)
         assert_equal(3, result["data"]["federation"]["bases"]["edges"].size)
-        assert_equal(true, result["data"]["federation"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is true when first is not specified")
+        assert_equal(false, result["data"]["federation"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is false when first is not specified")
       end
 
       it "applies to queries by `last`" do

spec/integration/rails/graphql/relay/array_connection_spec.rb

@@ -108,12 +108,11 @@ def get_page_info(result, key = "bases")
       result = star_wars_query(query_string, { "after" => first_cursor, "first" => 2 })
       assert_equal(["Y-Wing", "A-Wing"], get_names(result))
 
-      # After the last result, find the next 2:
-      second_cursor = get_last_cursor(result)
+      third_cursor = get_last_cursor(result)
 
-      # There is only 2 results between the cursors
-      result = star_wars_query(query_string, { "after" => first_cursor, "before" => second_cursor, "first" => 5 })
-      assert_equal(["Y-Wing", "A-Wing"], get_names(result))
+      # There is only 1 result between the cursors
+      result = star_wars_query(query_string, { "after" => first_cursor, "before" => third_cursor, "first" => 5 })
+      assert_equal(["Y-Wing"], get_names(result))
     end
 
     it 'handles cursors beyond the bounds of the array' do
@@ -175,7 +174,7 @@ def get_names(result)
         # Max page size is applied _without_ `first`, also
         result = star_wars_query(query_string)
         assert_equal(["Yavin", "Echo Base"], get_names(result))
-        assert_equal(true, get_page_info(result)["hasNextPage"], "hasNextPage is true when first is not specified")
+        assert_equal(false, get_page_info(result)["hasNextPage"], "hasNextPage is false when first is not specified")
       end
 
       it "applies to queries by `last`" do
@@ -235,7 +234,7 @@ def get_page_info(result)
         # Max page size is applied _without_ `first`, also
         result = star_wars_query(query_string)
         assert_equal(["Yavin", "Echo Base", "Secret Hideout"], get_names(result))
-        assert_equal(true, get_page_info(result)["hasNextPage"], "hasNextPage is true when first is not specified")
+        assert_equal(false, get_page_info(result)["hasNextPage"], "hasNextPage is false when first is not specified")
       end
 
       it "applies to queries by `last`" do

spec/integration/rails/graphql/relay/relation_connection_spec.rb

@@ -287,7 +287,7 @@ def get_last_cursor(result)
         # Max page size is applied _without_ `first`, also
         result = star_wars_query(query_string)
         assert_equal(2, result["data"]["empire"]["bases"]["edges"].size)
-        assert_equal(true, result["data"]["empire"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is true when first is not specified")
+        assert_equal(false, result["data"]["empire"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is false when first is not specified")
       end
 
       it "applies to queries by `last`" do
@@ -346,7 +346,7 @@ def get_last_cursor(result)
         # Max page size is applied _without_ `first`, also
         result = star_wars_query(query_string)
         assert_equal(3, result["data"]["empire"]["bases"]["edges"].size)
-        assert_equal(true, result["data"]["empire"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is true when first is not specified")
+        assert_equal(false, result["data"]["empire"]["bases"]["pageInfo"]["hasNextPage"], "hasNextPage is false when first is not specified")
       end
 
       it "applies to queries by `last`" do

spec/support/connection_assertions.rb

@@ -243,6 +243,23 @@ def assert_names(expected_names, result)
           refute get_page_info(res3, "hasPreviousPage")
         end
 
+        it "returns empty lists for `after: 1` and `before: 2`" do
+          res = exec_query(query_str, first: 2)
+          assert_names(["Avocado", "Beet"], res)
+          after_cursor = get_page_info(res, "startCursor")
+          before_cursor = get_page_info(res, "endCursor")
+
+          res = exec_query(query_str, after: after_cursor, before: before_cursor)
+          assert_equal true, get_page_info(res, "hasNextPage")
+          assert_equal true, get_page_info(res, "hasPreviousPage")
+          assert_names [], res
+
+          res = exec_query(query_str, after: after_cursor, before: before_cursor, first: 3)
+          assert_equal false, get_page_info(res, "hasNextPage")
+          assert_equal true, get_page_info(res, "hasPreviousPage")
+          assert_names [], res
+        end
+
         it "handles out-of-bounds cursors" do
           # It treats negative cursors like zero
           bogus_negative_cursor = NonceEnabledEncoder.encode("-10")
@@ -300,7 +317,7 @@ def assert_names(expected_names, result)
           res = exec_query(query_str, {})
           # Neither first nor last was provided, so default_page_size was applied.
           assert_names(["Avocado", "Beet", "Cucumber", "Dill"], res)
-          assert_equal true, get_page_info(res, "hasNextPage")
+          assert_equal false, get_page_info(res, "hasNextPage")
           assert_equal false, get_page_info(res, "hasPreviousPage")
         end