Merge branch 'allow-last-on-relations-with-keyset-order' into 'master'

Support `#last` on relations with keyset order

See merge request gitlab-org/gitlab!95915

config/initializers/active_record_keyset_pagination.rb

 # frozen_string_literal: true
 
 module PaginatorExtension
+  KEYSET_ORDER_PLACEHOLDER = Object.new
+
   # This method loads the records for the requested page and returns a keyset paginator object.
   def keyset_paginate(cursor: nil, per_page: 20, keyset_order_options: {})
     Gitlab::Pagination::Keyset::Paginator.new(scope: self.dup, cursor: cursor, per_page: per_page, keyset_order_options: keyset_order_options)
   end
+
+  # This modifies `reverse_sql_order` so that it is aware of Gitlab::Pagination::Keyset::Order which
+  # can reverse order clauses with NULLS LAST because we provide it a `reversed_order_expression`.
+  # This allows us to use `#last` on these relations.
+  #
+  # Overrides https://github.com/rails/rails/blob/v6.1.6.1/activerecord/lib/active_record/relation/query_methods.rb#L1331-L1358
+  def reverse_sql_order(order_query)
+    return super if order_query.empty?
+
+    keyset_order_values = []
+
+    order_query_without_keyset = order_query.flat_map do |o|
+      next o unless o.is_a?(Gitlab::Pagination::Keyset::Order)
+
+      keyset_order_values << o
+      KEYSET_ORDER_PLACEHOLDER
+    end
+
+    super(order_query_without_keyset).map do |o|
+      next o unless o == KEYSET_ORDER_PLACEHOLDER
+
+      keyset_order_values.shift.reversed_order
+    end
+  end
 end
 
 ActiveSupport.on_load(:active_record) do

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

@@ -116,7 +116,7 @@ def limited_nodes
               end
 
               if last
-                paginated_nodes = LastItems.take_items(sliced_nodes, limit_value + 1)
+                paginated_nodes = sliced_nodes.last(limit_value + 1)
 
                 # there is an extra node, so there is a previous page
                 @has_previous_page = paginated_nodes.count > limit_value

lib/gitlab/graphql/pagination/keyset/last_items.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Graphql
-    module Pagination
-      module Keyset
-        # This class handles the last(N) ActiveRecord call even if a special ORDER BY configuration is present.
-        # For the last(N) call, ActiveRecord calls reverse_order, however for some cases it raises
-        # ActiveRecord::IrreversibleOrderError error.
-        class LastItems
-          # rubocop: disable CodeReuse/ActiveRecord
-          def self.take_items(scope, count)
-            if Gitlab::Pagination::Keyset::Order.keyset_aware?(scope)
-              order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(scope)
-              items = scope.reorder(order.reversed_order).first(count)
-              items.is_a?(Array) ? items.reverse : items
-            else
-              scope.last(count)
-            end
-          end
-        end
-      end
-    end
-  end
-end

spec/lib/gitlab/graphql/pagination/keyset/last_items_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Graphql::Pagination::Keyset::LastItems do
-  let_it_be(:merge_request) { create(:merge_request) }
-
-  let(:scope) { MergeRequest.order_merged_at_asc }
-
-  subject { described_class.take_items(*args) }
-
-  context 'when the `count` parameter is nil' do
-    let(:args) { [scope, nil] }
-
-    it 'returns a single record' do
-      expect(subject).to eq(merge_request)
-    end
-  end
-
-  context 'when the `count` parameter is given' do
-    let(:args) { [scope, 1] }
-
-    it 'returns an array' do
-      expect(subject).to eq([merge_request])
-    end
-  end
-end