passing scope to url_for #172

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet

terralab commented Sep 2, 2011

this is a fix to allow for scope to be passed in to url_for, this is necessary for mountable engine support in rails 3.1. let me know if you have any questions
thanks,
adam

Owner

mislav commented Sep 2, 2011

Hey, I haven't used mountable engines yet. Can you explain a bit more how this works? Thanks!

terralab commented Sep 3, 2011

I assume you're familiar with engines. They're applications as plugins. Well in Rails 3.1 you can do this:

Compass3::Application.routes.draw do
  mount ErpApp::Engine => '/erp_app'
  mount Knitkit::Engine => '/knitkit'
end

You mount the engine and give it a path (scope) much like mounting a drive to your filesystem. Well what happens with url_for is the routes in that engine are scoped to the mounted path. The consequence is (using the example above) if I am in engine knitkit and and want to generate a url using a route in erp_app, by default I'll be in scope knitkit and url_for won't find the erp_app route. So, we have to do this:

erp_app.url_for(:use_route => :myroute)
and vice versa:
knitkit.url_for(:use_route => :myotherroute)

and for the root scope there is

main_app.url_for(:use_route => :mythirdroute)

hope this helps,
thanks,
adam

terralab commented Sep 3, 2011

one more thing: think of the scope as a route prefix or a namespace

NOTE: After more testing I discovered an issue with using the active_record_store session_store. The above works fine with cookie_store but if you use active_record_store "No route matches" is thrown on the second AJAX request after the second page load after resetting the session. The issue is likely with either rails routing and/or rails sessions as it seems to work with non-ajax requests and with cookie store. Very strange.

Also, you might want to do this as it makes the URL cleaner since the scope doesn't need to be passed with the request

if url_params[:scope]
scope = url_params[:scope]
url_params[:scope] = nil
return scope.url_for(url_params)
else

ok I think we fixed it. somehow :controller was being set from the session breaking the second request. So we delete it. There are now 2 commits in this push request. Here is the code:

if url_params[:scope]
      scope = url_params[:scope]
      url_params.delete(:scope)
      url_params.delete(:controller)
      scope.url_for(url_params)
    else
      @template.url_for(url_params)
    end

tvdeyen commented Dec 2, 2011

+1

tvdeyen commented Dec 2, 2011

@terralab could you please add documentation on how to use this fix?

I tried passing :scope => main_app to will_paginate helper. Not working.
I tried to pass :use_route => '/' to will_paginate helper. Not working.

What am I missing?

@mislav This is whole engine and mountable apps thing is THE next big thing in rails. So please add support for it. Thanks

terralab commented Dec 2, 2011

example:

<%= will_paginate @results, :class => 'pagination ajax', :params => { :widget_name => 'search',
                                                                        :widget_action => 'new',
                                                                        :uuid => @uuid,
                                                                        :query => params[:query],
                                                                        :content_type => params[:content_type],
                                                                        :section_permalink => params[:section_permalink],
                                                                        :per_page => params[:per_page],
                                                                        :only_path => true,
                                                                        :use_route => :widget,
                                                                        :scope => erp_app
                                                                      } %>

terralab commented Dec 2, 2011

:scope is the route object

tvdeyen commented Dec 2, 2011

Ah, this did the trick.
Great work.
Thanks!

@mislav: pretty please add this fix! It's working and needed :)

terralab commented Dec 2, 2011

Glad it works! Glad to help. One more note:
:use_route is the route name defined with :as in the route file

This patch is courtesy of the compass agile enterprise project:
https://github.com/portablemind/compass_agile_enterprise

hecbuma commented Aug 12, 2013

Are you gonna merge this fix or what?

@the-teacher the-teacher referenced this pull request in kaminari/kaminari Nov 5, 2013

Open

Rails 4, engines routing doesn't work #457

Owner

mislav commented Jun 18, 2014

Sorry for the late reply. I don't like about this fix that it prevents you from having a plain query parameter named scope. How about we move the :scope option from :params to the main options hash for will_paginate? Also maybe rename it to :url_scope or something less ambiguous?

For those who need a solution right now, you can subclass WillPaginate::ActionView::LinkRenderer and override the url() method so it calls url_for on the object you need.

kubum commented Dec 29, 2014

@mislav do you still want to accept another version of PR for this? i ran into this issue, seems like there is still no clean solution for this at the moment in master

@mislav - I too am hitting this due to engine namespaces. I was able to work around it by adding a custom link renderer and overriding the url method to add support for a custom :url_builder option like so:

    def url(page)
      @base_url_params ||= begin
                             url_params = merge_get_params(default_url_params)
                             merge_optional_params(url_params)
                           end

      url_params = @base_url_params.dup
      add_current_page_param(url_params, page)

      # Add optional url_builder support
      if @options[:url_builder]
        @options[:url_builder].call(url_params)
      else
        @template.url_for(url_params)
      end
    end

You can then specify a custom url builder via the will_paginate options like so:

    <%=
      will_paginate items, {
        url_builder: -> (params) { myengine.url_for(params) },
        renderer: CustomPaginationHelper::LinkRenderer,
        inner_window: 2
      }
    %>

This should allow customization for any edge-case url construction as well. If you'd like me to submit a pull request for this let me know.

This did the trick for me. I use will_paginate-bootstrap and am patching the renderer there to call my engine instead of @template. Not pretty, but works.

I don't know why i need to though, as it worked in 4.1 but stopped in 4.2. My engine is not isolated and maybe that does it (or i messed up)

Whatever happened to this request, it looked like it was going to make it but then didn't ?

Owner

mislav commented Dec 19, 2015

Whatever happened to this request, it looked like it was going to make it but then didn't ?

Yes I asked for an amended version of this PR, but never got it.

Hi @mislav , maybe that is because one doesn't need that anymore.
It worked for me in 4.1 and in 4.2 i had to add a helper into my base engine controller like

class OfficeController < ApplicationController
   helper  OfficeClerk::Engine.routes.url_helpers
   include OfficeClerk::Engine.routes.url_helpers

The include makes the urls available in the code, but the helper is needed to get it into the views.
With that all scoping is unneccessary.
I think in isolated engines this is taken care off.
So all is good, maybe some documentation, but this pr is not needed i think

sintro commented May 3, 2016

Surprisingly for me, that the possibility to pass the 'scope' option to will_paginate method is not implemented yet...

@dancinglightning, this didnt work for me. In my case, the controller which renders the page having will_paginate belongs to engine, but the route is defined in main_app. Of course, will_paginate tries to get url by current engines scope, and fails. So I tried to use your advice, but included the main_app:

   helper  Rails.application.routes.url_helpers
   include Rails.application.routes.url_helpers

This didnt work. Temporarily solved my problem by defining route in engines class (right in my common application`s routes.rb:

Refinery::Core::Engine.routes.draw do
  namespace :blog, :path => Refinery::Blog.page_url do
    get 'search', to: 'posts#search', as: 'search'
  end
end

)
This seems to me more organic way to solve the problem, but it is ugly too, because this is class opening using and there is a lack of route ordering control.

@sintro not sure, but, I think, kaminari has the same problem.

JohnMerlino1 commented Jul 22, 2016

Nothing above has worked for me at all. I am using a Rails 4 app with Fullcalendar::Engine.
I have the following in my routes.rb file:

  mount FullcalendarEngine::Engine , at: "/fullcalendar_engine"

  resources :events,  module: 'fullcalendar_engine' , only: [:index, :show] do

    collection do
      get 'calendar', to: 'events#calendar', as: 'calendar'
      post :check
   end
end

I tried this:

<%= will_paginate @events, {
        renderer: CustomLinkRenderer,
        url_builder: -> (params) { FullcalendarEngine::Engine.routes.url_for(params) },
        params: { controller: 'events', action: :index, document_type: 'Contact', document_id: params[:id] }
    }

I created the CustomLinkRenderer in /lib:

class CustomLinkRenderer < WillPaginate::ActionView::LinkRenderer
  def url(page)
    @base_url_params ||= begin
    url_params = merge_get_params(default_url_params)
    url_params[:only_path] = true
    merge_optional_params(url_params)
  end

    url_params = @base_url_params.dup
    add_current_page_param(url_params, page)

   if @options[:url_builder]
     @options[:url_builder].call(url_params)
   else
     @template.url_for(url_params)
    end
  end
end

And I still get the following error:

No route matches {:action=>"index", :contactable_type=>"Lead", :controller=>"events", :document_id=>"5760535f4d6163133e020000", :document_type=>"Contact", :id=>"5760535f4d6163133e020000", :page=>2}

Can anyone offer me any kind of solution that will work?

@JohnMerlino1 JohnMerlino1 referenced this pull request in vinsol/fullcalendar-rails-engine Jul 22, 2016

Open

Did anyone get this working with will_paginate? #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment