Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

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

#197

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 .

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

State: resolved
Milestone: none
Assigned to: Mislav Mislav

Activity

  1. Mislav
    Mislav
    • State changed from new to open

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

    March 4th, 2008 @ 08:08 PM

  2. Yuri
    Yuri

    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
    

    March 9th, 2008 @ 04:48 PM

  3. Yuri
    Yuri

    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 
    

    March 9th, 2008 @ 04:52 PM

  4. Mislav
    Mislav
    • State changed from open to resolved

    April 5th, 2008 @ 04:29 AM

  5. Mislav

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.