Merge pull request #2845 from ptsavdar/fix-has_previous-has_next_page

Fix has_previous and has_next page values when sliced_nodes has not been called
rmosolgo · Mar 25, 2020 · 09a8ec4 · 09a8ec4
2 parents 20fe611 + ec9c24c
commit 09a8ec4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 20 deletions.
diff --git a/lib/graphql/pagination/relation_connection.rb b/lib/graphql/pagination/relation_connection.rb
@@ -12,7 +12,7 @@ def nodes
 
       def has_previous_page
         if @has_previous_page.nil?
-          @has_previous_page = if @after_offset && @after_offset > 0
+          @has_previous_page = if after_offset && after_offset > 0
             true
           elsif last
             # See whether there are any nodes _before_ the current offset.
@@ -29,7 +29,7 @@ def has_previous_page
 
       def has_next_page
         if @has_next_page.nil?
-          @has_next_page = if @before_offset && @before_offset > 0
+          @has_next_page = if before_offset && before_offset > 0
             true
           elsif first
             relation_count(set_limit(sliced_nodes, first + 1)) == first + 1
@@ -105,33 +105,41 @@ def set_limit(relation, limit_value)
       def sliced_nodes
         @sliced_nodes ||= begin
           paginated_nodes = items
-          @after_offset = after && offset_from_cursor(after)
-          @before_offset = before && offset_from_cursor(before)
 
-          if @after_offset
+          if after_offset
             previous_offset = relation_offset(items) || 0
-            paginated_nodes = set_offset(paginated_nodes, previous_offset + @after_offset)
+            paginated_nodes = set_offset(paginated_nodes, previous_offset + after_offset)
           end
 
-          if @before_offset && @after_offset
-            if @after_offset < @before_offset
+          if before_offset && after_offset
+            if after_offset < before_offset
               # Get the number of items between the two cursors
-              space_between = @before_offset - @after_offset - 1
+              space_between = before_offset - after_offset - 1
               paginated_nodes = set_limit(paginated_nodes, space_between)
             else
               # TODO I think this is untested
               # The cursors overextend one another to an empty set
               paginated_nodes = null_relation(paginated_nodes)
             end
-          elsif @before_offset
+          elsif before_offset
             # Use limit to cut off the tail of the relation
-            paginated_nodes = set_limit(paginated_nodes, @before_offset - 1)
+            paginated_nodes = set_limit(paginated_nodes, before_offset - 1)
           end
 
           paginated_nodes
         end
       end
 
+      # @return [Integer, nil]
+      def before_offset
+        @before_offset ||= before && offset_from_cursor(before)
+      end
+
+      # @return [Integer, nil]
+      def after_offset
+        @after_offset ||= after && offset_from_cursor(after)
+      end
+
       # Apply `first` and `last` to `sliced_nodes`,
       # returning a new relation
       def limited_nodes

diff --git a/spec/integration/rails/graphql/relay/page_info_spec.rb b/spec/integration/rails/graphql/relay/page_info_spec.rb
@@ -23,14 +23,14 @@ def get_last_cursor(result)
     query getShips($first: Int, $after: String, $last: Int, $before: String, $nameIncludes: String){
       empire {
         bases(first: $first, after: $after, last: $last, before: $before, nameIncludes: $nameIncludes) {
-          edges {
-            cursor
-          }
           pageInfo {
             hasNextPage
             hasPreviousPage
             startCursor
             endCursor
+          },
+          edges {
+            cursor
           }
         }
       }

diff --git a/spec/integration/rails/graphql/relay/relation_connection_spec.rb b/spec/integration/rails/graphql/relay/relation_connection_spec.rb
@@ -30,18 +30,18 @@ def get_last_cursor(result)
       }
 
       fragment basesConnection on BasesConnectionWithTotalCount {
+        pageInfo {
+          hasNextPage
+          hasPreviousPage
+          startCursor
+          endCursor
+        },
         totalCount,
         edges {
           cursor
           node {
             name
           }
-        },
-        pageInfo {
-          hasNextPage
-          hasPreviousPage
-          startCursor
-          endCursor
         }
       }
     |}