Merge pull request #231 from gfmurphy/fix-pagination-links

Use Query#params in pagination links instead of Query#hash.

Author: richmolj

lib/graphiti/delegates/pagination.rb

@@ -20,13 +20,17 @@ def links
 
       private
 
+      def pagination_params
+        @pagination_params ||= @proxy.query.params.reject { |key, _| [:action, :controller, :format].include?(key) }
+      end
+
       def pagination_link(page)
         return nil unless @proxy.resource.endpoint
 
         uri = URI(@proxy.resource.endpoint[:url].to_s)
 
         # Overwrite the pagination query params with the desired page
-        uri.query = @proxy.query.hash.merge({
+        uri.query = pagination_params.merge({
           page: {
             number: page,
             size: page_size

spec/delegates/pagination_spec.rb

@@ -53,6 +53,32 @@
         expect(subject[:first]).to eq(pagination_link(1, size: Graphiti::Scoping::Paginate::DEFAULT_PAGE_SIZE))
       end
     end
+
+    context "with included relationship" do
+      let(:params) {
+        {include: "bar,bazzes", filter: {"bazzes.deprecated" => "foo"}}
+      }
+
+      it "preserves include directive and filters on relationships" do
+        query = URI.decode_www_form(URI(subject[:first]).query).to_h
+        expect(query["include"]).to include("bar")
+        expect(query["include"]).to include("bazzes")
+        expect(query["filter[bazzes.deprecated]"]).to eq("foo")
+      end
+    end
+
+    context "with rails parameters" do
+      let(:params) {
+        {controller: "foos", action: "index", format: "jsonapi"}
+      }
+
+      it "removes them" do
+        page_links = subject.values
+        expect(page_links).to all(satisfy { |v| v["controller=foos"].nil? })
+        expect(page_links).to all(satisfy { |v| v["action=index"].nil? })
+        expect(page_links).to all(satisfy { |v| v["format=jsonapi"].nil? })
+      end
+    end
   end
 
   def pagination_link(number, size: current_per_page)

spec/support/pagination_link_helper.rb

@@ -2,11 +2,30 @@
 
 RSpec.shared_context "pagination_context", shared_context: :metadata do
   let(:proxy) do
-    double(resource: resource, query: query, scope: scope)
     Graphiti::ResourceProxy.new(resource, scope, query)
   end
-  let(:resource) { double(endpoint: endpoint, default_page_size: nil) }
-  let(:query) { double(hash: params) }
+
+  let(:resource) do
+    Class.new(Graphiti::Resource) do
+      attribute :deprecated, :integer, filterable: true
+      belongs_to :bar, resource: self
+      has_many :bazzes, resource: self
+
+      def self.endpoint
+        {
+          path: "/foos",
+          full_path: "/api/v2/foos",
+          url: "http://localhost:3000/api/v2/foos",
+          actions: [:index, :show, :create, :update, :destroy]
+        }
+      end
+
+      def self.name
+        "foos"
+      end
+    end.new
+  end
+  let(:query) { Graphiti::Query.new(resource, params) }
   let(:scope) { double(object: collection, pagination: double(size: current_per_page)) }
   let(:pagination_delegate) { Graphiti::Delegates::Pagination.new(proxy) }
   let(:collection) do
@@ -33,11 +52,6 @@
     }
   end
   let(:endpoint) do
-    {
-      path: "/foos",
-      full_path: "/api/v2/foos",
-      url: "http://localhost:3000/api/v2/foos",
-      actions: [:index, :show, :create, :update, :destroy]
-    }
+    resource.endpoint
   end
 end