#197 ✓resolved
Yuri

using :param_name => "something[page]" leads to invalid behavior

Reported by Yuri | March 4th, 2008 @ 06:47 PM

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

    Mislav March 4th, 2008 @ 08:08 PM

    • State changed from “new” to “open”

    Thanks for the detailed report! I'll take a deeper look as soon as I'm able.

  • Yuri

    Yuri March 9th, 2008 @ 04:48 PM

    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
end

    and 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

    Yuri March 9th, 2008 @ 04:52 PM

    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

    Mislav April 5th, 2008 @ 04:29 AM

    • State changed from “open” to “resolved”
  • Mislav

    Mislav January 31st, 2012 @ 11:18 PM

    • Tag set to “view, will_paginate”
    • Milestone order changed from “0” to “0”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Everyone's favorite Ruby library for pagination of practically anything!

People watching this ticket

Tags

Pages