Merge branch '63778-graphql-keyset-is-null' into 'master'

Graphql: Keyset::Connection to support "attribute ASC NULLS LAST"

See merge request gitlab-org/gitlab!18916

lib/gitlab/graphql/connections/keyset/connection.rb

@@ -8,10 +8,15 @@
 # https://coderwall.com/p/lkcaag/pagination-you-re-probably-doing-it-wrong
 #
 # It currently supports sorting on two columns, but the last column must
-# be the primary key. For example
+# be the primary key. If it's not already included, an order on the
+# primary key will be added automatically, like `order(id: :desc)`
 #
 #   Issue.order(created_at: :asc).order(:id)
-#   Issue.order(due_date: :asc).order(:id)
+#   Issue.order(due_date: :asc)
+#
+# You can also use `Gitlab::Database.nulls_last_order`:
+#
+#   Issue.reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC'))
 #
 # It will tolerate non-attribute ordering, but only attributes determine the cursor.
 # For example, this is legitimate:

lib/gitlab/graphql/connections/keyset/order_info.rb

@@ -5,12 +5,15 @@ module Gitlab
     module Connections
       module Keyset
         class OrderInfo
+          attr_reader :attribute_name, :sort_direction
+
           def initialize(order_value)
-            @order_value = order_value
+            if order_value.is_a?(String)
+              @attribute_name, @sort_direction = extract_nulls_last_order(order_value)
+            else
+              @attribute_name = order_value.expr.name
+              @sort_direction = order_value.direction
             end
-
-          def attribute_name
-            order_value.expr.name
           end
 
           def operator_for(before_or_after)
@@ -22,10 +25,10 @@ module Gitlab
             end
           end
 
-          # Only allow specific node types.  For example ignore String nodes
+          # Only allow specific node types
           def self.build_order_list(relation)
             order_list = relation.order_values.select do |value|
-              value.is_a?(Arel::Nodes::Ascending) || value.is_a?(Arel::Nodes::Descending)
+              supported_order_value?(value)
             end
 
             order_list.map { |info| OrderInfo.new(info) }
@@ -52,12 +55,21 @@ module Gitlab
             end
           end
 
+          def self.supported_order_value?(order_value)
+            return true if order_value.is_a?(Arel::Nodes::Ascending) || order_value.is_a?(Arel::Nodes::Descending)
+            return false unless order_value.is_a?(String)
+
+            tokens = order_value.downcase.split
+
+            tokens.last(2) == %w(nulls last) && tokens.count == 4
+          end
+
           private
 
-          attr_reader :order_value
+          def extract_nulls_last_order(order_value)
+            tokens = order_value.downcase.split
 
-          def sort_direction
-            order_value.direction
+            [tokens.first, (tokens[1] == 'asc' ? :asc : :desc)]
           end
         end
       end

spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb

@@ -17,6 +17,26 @@ describe Gitlab::Graphql::Connections::Keyset::OrderInfo do
         expect(order_list.last.operator_for(:after)).to eq '>'
       end
     end
+
+    context 'when order contains NULLS LAST' do
+      let(:relation) { Project.order(Arel.sql('projects.updated_at Asc Nulls Last')).order(:id) }
+
+      it 'does not ignore the SQL order' do
+        expect(order_list.count).to eq 2
+        expect(order_list.first.attribute_name).to eq 'projects.updated_at'
+        expect(order_list.first.operator_for(:after)).to eq '>'
+        expect(order_list.last.attribute_name).to eq 'id'
+        expect(order_list.last.operator_for(:after)).to eq '>'
+      end
+    end
+
+    context 'when order contains invalid formatted NULLS LAST ' do
+      let(:relation) { Project.order(Arel.sql('projects.updated_at created_at Asc Nulls Last')).order(:id) }
+
+      it 'ignores the SQL order' do
+        expect(order_list.count).to eq 1
+      end
+    end
   end
 
   describe '#validate_ordering' do