fixes for pagy_url_for full url gets duplicated page param (#149)

- used concatenated request.base_url instead of request.url
- added better tests
- fixed overriding of pagy_url_for in items extra

lib/pagy/extras/items.rb

@@ -38,9 +38,10 @@ def pagy#{prefix}_get_vars_with_items(collection, vars)
   module Frontend
 
     alias_method :pagy_url_for_without_items, :pagy_url_for
-    def pagy_url_for_with_items(page, pagy)
-      p_vars = pagy.vars; params = request.GET.merge(p_vars[:page_param] => page, p_vars[:items_param] => p_vars[:items]).merge!(p_vars[:params])
-      "#{request.path}?#{Rack::Utils.build_nested_query(pagy_get_params(params))}#{p_vars[:anchor]}"
+    def pagy_url_for_with_items(page, pagy, url=false)
+      p_vars = pagy.vars; params = request.GET.merge(p_vars[:params]); params[p_vars[:page_param].to_s] = page
+      params[p_vars[:items_param].to_s] = p_vars[:items]
+      "#{request.base_url if url}#{request.path}?#{Rack::Utils.build_nested_query(pagy_get_params(params))}#{p_vars[:anchor]}"
     end
     alias_method :pagy_url_for, :pagy_url_for_with_items
 

lib/pagy/frontend.rb

@@ -12,9 +12,9 @@ class Pagy
 
   module Helpers
     # This works with all Rack-based frameworks (Sinatra, Padrino, Rails, ...)
-    def pagy_url_for(page, pagy, path_or_url=:path)
+    def pagy_url_for(page, pagy, url=false)
       p_vars = pagy.vars; params = request.GET.merge(p_vars[:params]); params[p_vars[:page_param].to_s] = page
-      "#{request.send(path_or_url)}?#{Rack::Utils.build_nested_query(pagy_get_params(params))}#{p_vars[:anchor]}"
+      "#{request.base_url if url}#{request.path}?#{Rack::Utils.build_nested_query(pagy_get_params(params))}#{p_vars[:anchor]}"
     end
 
     # Sub-method called only by #pagy_url_for: here for easy customization of params by overriding

test/pagy/extras/items_test.rb

@@ -195,7 +195,7 @@
 
     it 'renders url with params and anchor' do
       pagy = Pagy.new count: 1000, page: 3, params: {a: 3, b: 4}, anchor: '#anchor', items: 40
-      frontend.pagy_url_for(5, pagy).must_equal '/foo?page=5&items=40&a=3&b=4#anchor'
+      frontend.pagy_url_for(5, pagy).must_equal "/foo?page=5&a=3&b=4&items=40#anchor"
     end
 
   end

test/pagy/extras/trim_test.rb

@@ -38,7 +38,7 @@
       pagy = Pagy.new(count: 1000, params: {a:3,b:4})
       link = frontend.pagy_link_proc(pagy)
       link.call(1).must_equal("<a href=\"/foo?a=3&b=4\"   >1</a>")
-      link.call(10).must_equal("<a href=\"/foo?a=3&b=4&page=10\"   >10</a>")
+      link.call(10).must_equal("<a href=\"/foo?page=10&a=3&b=4\"   >10</a>")
     end
 
   end

test/pagy/frontend_test.rb

@@ -184,21 +184,25 @@
     it 'renders basic url' do
       pagy = Pagy.new count: 1000, page: 3
       frontend.pagy_url_for(5, pagy). must_equal '/foo?page=5'
+      frontend.pagy_url_for(5, pagy, :url). must_equal 'http://example.com:3000/foo?page=5'
     end
 
     it 'renders url with params' do
       pagy = Pagy.new count: 1000, page: 3, params: {a: 3, b: 4}
-      frontend.pagy_url_for(5, pagy).must_equal "/foo?a=3&b=4&page=5"
+      frontend.pagy_url_for(5, pagy).must_equal "/foo?page=5&a=3&b=4"
+      frontend.pagy_url_for(5, pagy, :url).must_equal "http://example.com:3000/foo?page=5&a=3&b=4"
     end
 
     it 'renders url with anchor' do
       pagy = Pagy.new count: 1000, page: 3, anchor: '#anchor'
       frontend.pagy_url_for(6, pagy).must_equal '/foo?page=6#anchor'
+      frontend.pagy_url_for(6, pagy, :url).must_equal 'http://example.com:3000/foo?page=6#anchor'
     end
 
     it 'renders url with params and anchor' do
       pagy = Pagy.new count: 1000, page: 3, params: {a: 3, b: 4}, anchor: '#anchor'
-      frontend.pagy_url_for(5, pagy).must_equal "/foo?a=3&b=4&page=5#anchor"
+      frontend.pagy_url_for(5, pagy).must_equal "/foo?page=5&a=3&b=4#anchor"
+      frontend.pagy_url_for(5, pagy, :url).must_equal "http://example.com:3000/foo?page=5&a=3&b=4#anchor"
     end
 
   end
@@ -208,7 +212,7 @@
     it 'overrides params' do
       overridden = TestViewOverride.new
       pagy = Pagy.new count: 1000, page: 3, params: {a: 3, b: 4}, anchor: '#anchor'
-      overridden.pagy_url_for(5, pagy).must_equal "/foo?b=4&page=5&k=k#anchor"
+      overridden.pagy_url_for(5, pagy).must_equal "/foo?page=5&b=4&k=k#anchor"
     end
 
   end

test/test_helper/backend.rb

@@ -14,7 +14,7 @@ def initialize(params={a: 'a', page: 3})
   end
 
   def request
-    @request ||= Rack::Request.new('SCRIPT_NAME' => '/foo','HTTPS' => 'on', 'HTTP_HOST' => 'example.com:8080')
+    @request ||= Rack::Request.new(Rack::MockRequest.env_for('https://example.com:8080/foo?page=1'))
   end
 
   def response

test/test_helper/frontend.rb

@@ -5,7 +5,7 @@ class TestView
   include Pagy::Frontend
 
   def request
-    Rack::Request.new('SCRIPT_NAME' => '/foo')
+    Rack::Request.new(Rack::MockRequest.env_for('http://example.com:3000/foo?page=2'))
   end
 end