using :param_name => "something[page]" leads to invalid behavior
Reported by yuri.leikind (at gmail) | March 4th, 2008 @ 09:47 AM
Problem
If I use a compound parameter name for :param_name, like this:
will_paginate(resultset, :param_name => "test[page]")
After a couple of clicks on page links in the pagination panel it gets stuck on one page, no matter where you click
Why
will_paginate preserves GET parameters for page links (which is cool). This is done in view_helpers.rb :
def url_options(page)
options = { param_name => page }
# page links should preserve GET parameters
options = params.merge(options) if @template.request.get?
options.rec_merge!(@options[:params]) if @options[:params]
return options
end
The problem is that with a parameter name like test[page] before merging optons and params the options has is
{:"test[page]"=>1}
while params also contain this parameter if the user has already clicked once on a page link, but it's parsed into a hash of hashes:
{:action=>"index", :controller=>"dashboards", :test=>{"page"=>"2"}}
Consequently, params.merge(options) does not overwrite :test=>{"page"=>"2"}, the resulting merged hash is
{:test=>{"page"=>"2"}, :action=>"index", :controller=>"dashboards", :"test[page]"=>1}
and in the URL the page parameter is repeated twice:
?test%5Bpage%5D=1&test%5Bpage%5D=2
Solution
I fixed it with the following code:
def url_options(page)
options = ActionController::AbstractRequest.parse_query_parameters("#{param_name}=#{page}")
options.keys.each do |key| # convert keys to symbols
options[key.intern] = options[key]
options.delete(key)
end
# page links should preserve GET parameters
options = params.merge(options) if @template.request.get?
options.rec_merge!(@options[:params]) if @options[:params]
return options
end
There might be a more elegant way, but this one works .
Comments and changes to this ticket
-
Mislav March 4th, 2008 @ 11:08 AM
- → State changed from “new” to “open”
Thanks for the detailed report! I'll take a deeper look as soon as I'm able.
-
yuri.leikind (at gmail) March 9th, 2008 @ 08:48 AM
Hey,
Just realized that my initial solution is not ideal.
Imagine that GET parameters were
grid[:order] = 'created_at' grid[:order_direction] = 'asc'and the page parameter name is grid[:page].
The code above (line options = params.merge(options) if @template.request.get?) will overwrite grid[:order] and grid[:order_direction], so we need recursive merge.
I added a non destructive rec_merge (core_ext.rb):
unless Hash.instance_methods.include? 'rec_merge' Hash.class_eval do def rec_merge(other) res = self.clone other.each do |key, other_value| value = res[key] if value.is_a?(Hash) and other_value.is_a?(Hash) self[key] = value.rec_merge other_value else self[key] = other_value end end res end end endand the final working version is
def url_options(page) options = ActionController::AbstractRequest.parse_query_parameters("#{param_name}=#{page}") options.keys.each do |key| options[key.intern] = options[key] options.delete(key) end options = params.rec_merge(options) if @template.request.get? options.rec_merge!(@options[:params]) if @options[:params] return options end -
yuri.leikind (at gmail) March 9th, 2008 @ 08:52 AM
Sunday is a bad day for coding, please ignore def rec_merge(other) above, sorry, the correct version is:
def rec_merge(other) res = self.clone other.each do |key, other_value| value = res[key] if value.is_a?(Hash) and other_value.is_a?(Hash) res[key] = value.rec_merge other_value else res[key] = other_value end end res end -
Mislav April 4th, 2008 @ 07:29 PM
- → State changed from “open” to “resolved”
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
