New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveAdmin creates an extra root route #2049

Closed
jbhannah opened this Issue Apr 4, 2013 · 44 comments

Comments

Projects
None yet
@jbhannah
Copy link
Contributor

jbhannah commented Apr 4, 2013

Abbreviated output of rake routes:

admin_root        /admin(.:format) admin/dashboard#index
      root        /                dashboard#index
(snip)
      root        /                home#index

Abbreviated contents of routes.rb:

RailsAppName::Application.routes.draw do
  ActiveAdmin.routes(self)
(snip)
  root to: "home#index"
end

A sample error while running specs:

ActionController::RoutingError: uninitialized constant DashboardController

I don't have a controller named DashboardController. The only other Rails engine I'm using is Devise, which doesn't have a DashboardController. This is a new behavior with 0.6.0, having made no changes other than updating my Gemfile to the new version from 0.5.1.

@jbhannah

This comment has been minimized.

Copy link
Contributor Author

jbhannah commented Apr 4, 2013

If I comment out everything in routes.rb except the ActiveAdmin line and the root to: line, I still get the duplicate root route. All other ActiveAdmin routes come after the incorrect root (under the snip in the above rake routes output).

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Apr 4, 2013

I'm not sure what's causing this.
A temporary workaround is to specify your own routes before AA routes, so they have higher priority.

RailsAppName::Application.routes.draw do
  root to: "home#index"
  ActiveAdmin.routes(self)
end
@jbhannah

This comment has been minimized.

Copy link
Contributor Author

jbhannah commented Apr 4, 2013

My best guess is that it was a change made in f8b067e, possibly in the define_basic_routes or root_and_dashboard_routes methods of lib/active_admin/router.rb. It seems like line 37 (as of current HEAD) of that file is being applied to the application's base router, when it shouldn't be.

@exviva

This comment has been minimized.

Copy link
Contributor

exviva commented Apr 4, 2013

I can confirm this bug, just upgraded to 0.6.0, getting an ActionController::RoutingError: uninitialized constant DashboardController error. The workaround suggested by @daxter worked.

@henrrrik

This comment has been minimized.

Copy link
Contributor

henrrrik commented Apr 4, 2013

Just ran into this too. Workaround did the trick.

@marclennox

This comment has been minimized.

Copy link

marclennox commented Apr 4, 2013

Me too. Had to move my root route above active_admin routes otherwise mine gets clobbered by AA.

@marcusg

This comment has been minimized.

Copy link
Contributor

marcusg commented Apr 4, 2013

works for me with the workaround 👍

@marcusg

This comment has been minimized.

Copy link
Contributor

marcusg commented Apr 4, 2013

But there is another problem: using the workaround causing an endless loop.

the generated routes:

root                    /                                        pages#welcome
admin_root          /admin(.:format)                 admin/dashboard#index
root                    /                                        dashboard#index

when accessing http:://localhost:3000/admin I'll get net::ERR_TOO_MANY_REDIRECTS from Chrome

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Apr 4, 2013

I assume the scenario is you're running AA under the /admin namespace with no dashboard set up, as well as having a root to: 'somewhere'? Because I can't reproduce this problem.

In that scenario, AA builds admin_root as you would expect, as well as the manual root you defined.

admin_root    /admin(.:format)         admin/dashboard#index
...
root          /                        foo#bar

I should mention that admin_root is configurable in the initializer:

  # == Root
  #
  # Set the action to call for the root path. You can set different
  # roots for each namespace.
  #
  # Default:
  # config.root_to = 'dashboard#index'
@excid3

This comment has been minimized.

Copy link

excid3 commented Apr 4, 2013

Workaround works(-ish) for me. When logging in, I get taken back to the app's root, and not ActiveAdmin though. Might be related.

@jbhannah

This comment has been minimized.

Copy link
Contributor Author

jbhannah commented Apr 4, 2013

The workaround works for me, but admin comments are causing other problems if you have a separate Comment model and CommentsController (see #301). Basically, the same thing is happening with the admin comments routes as with the admin root route: they're being duplicated in the Rails app's root namespaces in addition to their normal place in ActiveAdmin's namespace. (Update, a day later: PR #2059 fixes this conflict, but the routes are still being duplicated.)

@lupinglade

This comment has been minimized.

Copy link
Contributor

lupinglade commented Apr 5, 2013

Same as #2053

@marcusg

This comment has been minimized.

Copy link
Contributor

marcusg commented Apr 5, 2013

@daxter; I have a dashboard using the new dashboard syntax with ActiveAdmin.register_page "Dashboard" do. I tried to change the admin_root in the initializer, but with no effect. I still can access all admin pages, except the /admin due to many redirects.

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Apr 5, 2013

@marcusg it's not admin_root you need to set in the initializer, but root_to.

@marcusg

This comment has been minimized.

Copy link
Contributor

marcusg commented Apr 5, 2013

@daxter: sorry, I meant root_to. Any other ideas? Can I help posting some of my env vars?

@jbhannah

This comment has been minimized.

Copy link
Contributor Author

jbhannah commented Apr 5, 2013

Okay, so here's what I've found: the root namespace was being added to the list of namespaces, even when nothing in ActiveAdmin was using the root namespace. This turned out to be due to a value of nil or false being passed to ActiveAdmin::Application#namespace(name), so I though it would be fixed by having that method use default_namespace instead of :root when nil or false is passed as name, but :root turns out to be the expected value to be used in that case (my fault for not testing well enough before sending a PR). So that wasn't it.

Going back a bit further, though, only in ActiveAdmin::Application#load! (when running in my application, which doesn't have ActiveAdmin use the root namespace at all) was nil or false being passed to that function. Looking at the history, that behavior was changed in cd09ea0: load! used to effectively run namespace(default_namespace) (through another function and under a different name). Restoring the original behavior of loading the configured default namespace, instead of loading the root namespace by omission, when loading the application fixes the problem of duplicate root routes, with all tests passing. Will be submitting a new pull request shortly.

@marcusg

This comment has been minimized.

Copy link
Contributor

marcusg commented Apr 6, 2013

@jbhannah: tried your branch and it's working fine! thanks, hope it get's merged fast. @daxter: the problem with too many redirects was cancan related. I did not set up the cancan abilities correctly... can :read, ActiveAdmin::Page, :name => "Dashboard" was missing. cheers!

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Apr 7, 2013

If a few more people can confirm that #2062 fixes the problem, I'll pull it in.

@lupinglade

This comment has been minimized.

Copy link
Contributor

lupinglade commented Apr 7, 2013

Yes that fixed it.

@climber2002

This comment has been minimized.

Copy link

climber2002 commented Apr 7, 2013

got this issue to and the workaround works!

@jbhannah

This comment has been minimized.

Copy link
Contributor Author

jbhannah commented Apr 7, 2013

@climber2006 Did you test just the workaround of moving ActiveAdmin's route entry below your root route, or did you try the fix in #2062?

@ghost

This comment has been minimized.

Copy link

ghost commented Apr 8, 2013

@jbhannah I just put my root before ActiveAdmin's route and I didn't try the fix yet.

@sfeu

This comment has been minimized.

Copy link

sfeu commented Apr 8, 2013

i would vote for a new version to publish this patch

benben pushed a commit to benben/active_admin that referenced this issue Apr 8, 2013

@optimum-dulopin

This comment has been minimized.

Copy link

optimum-dulopin commented Apr 9, 2013

+1 for new version ;-)

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Apr 9, 2013

There are a couple other things we'd like to resolve before 0.6.1

@danoffd

This comment has been minimized.

Copy link

danoffd commented Apr 12, 2013

i'm clicking in circles trying to figure out what the workaround is. anybody want to help a newbie out?

on edit.... got it,

on my drive, manually edit this file:
C:\Ruby193\lib\ruby\gems\1.9.1\gems\activeadmin-0.6.0\lib\active_admin\application.rb

replace line 180
namespace(nil) # init AA resources
with...
namespace(default_namespace) # init AA resources

Im guessing its a bad idea to deploy to heroku with this bug, but gonna do it anyway

@jbhannah

This comment has been minimized.

Copy link
Contributor Author

jbhannah commented Apr 12, 2013

@danoffd You've only changed the local copy of your gem, it won't be in place on Heroku when you deploy since Heroku does a pristine install of every gem in your Gemfile. Until 0.6.1 is released, you can change the active_admin line in your Gemfile to

gem "activeadmin", "~> 0.6.0", :git => "git://github.com/gregbell/active_admin.git"

then run bundle update activeadmin. Then, locally and on Heroku, the gem will be installed from the master Git branch. When 0.6.1 is released, simply remove the :git part of the line and bundle update activeadmin again.

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Apr 12, 2013

The cleaner syntax being:

gem 'activeadmin', github: 'gregbell/active_admin'
@danoffd

This comment has been minimized.

Copy link

danoffd commented Apr 12, 2013

yall rock

@joerails

This comment has been minimized.

Copy link

joerails commented Apr 15, 2013

still not working in my windows Rails app.kindly help

@jpmckinney

This comment has been minimized.

Copy link
Contributor

jpmckinney commented May 15, 2013

There should be a hotfix for this issue: breaking the host app's root route makes AA unusable in most cases.

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented May 15, 2013

Not a hotfix, but there's a workaround. What we should really do is release a new version.

@mjobin-mdsol

This comment has been minimized.

Copy link

mjobin-mdsol commented Jun 4, 2013

I encountered an extra problem related with this. I wonder if the fix in 0.6.1 will cover it.

I tried to move down the ActiveAdmin.routes(self) under a scope "foo" {} block

and from that point, root_url returns the active_admin extra root url.

will this be fixed in 0.6.1 ?

mjobin-mdsol pushed a commit to mjobin-mdsol/active_admin that referenced this issue Jun 4, 2013

@jbhannah

This comment has been minimized.

Copy link
Contributor Author

jbhannah commented Jun 4, 2013

@mjobin-mdsol: Try updating your Gemfile to use the master branch of ActiveAdmin for now and see if the problem still exists.

@mjobin-mdsol

This comment has been minimized.

Copy link

mjobin-mdsol commented Jun 4, 2013

I created a branch in my fork based on 0.6.0 with the fix include in PR #2062

https://github.com/mjobin-mdsol/active_admin/compare/0.6.0-p1

feel free to use in your gemfile using

gem 'activeadmin', github: 'mjobin-mdsol/active_admin', :branch => '0.6.0-p1' # "~> 0.6.1"

@mjobin-mdsol

This comment has been minimized.

Copy link

mjobin-mdsol commented Jun 4, 2013

@jbhannah thanks ! I confirm, the fix in #2062 fixes the problem with scope too. works very well.

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Jun 4, 2013

What scope problem are you talking about?

@mjobin-mdsol

This comment has been minimized.

Copy link

mjobin-mdsol commented Jun 5, 2013

the question I asked a little earlier.

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Jun 5, 2013

Oh, I see

@vhbsouza

This comment has been minimized.

Copy link

vhbsouza commented Jun 12, 2013

I was with #2053 issue and just by changing root :to => 'posts#index' order in the routes.rb file worked for me.

@rempargo

This comment has been minimized.

Copy link

rempargo commented Jul 12, 2013

Why does this problem only occur under production environment?

@mjobin-mdsol

This comment has been minimized.

Copy link

mjobin-mdsol commented Jul 12, 2013

I believe it happened with me in development as well

@brain-geek

This comment has been minimized.

Copy link

brain-geek commented Sep 22, 2013

Having the same problem. It would be very nice to release 0.6.1 .

@seanlinsley

This comment has been minimized.

Copy link
Member

seanlinsley commented Sep 22, 2013

@gregbell should be releasing 0.6.1 in the next couple days

@chuckweinberger

This comment has been minimized.

Copy link

chuckweinberger commented Sep 25, 2013

I was having this problem in development as well as production (on Heroku). Couldn't simply swap route order because of other routing issues. Can confirm that the newly released 0.6.1 has fixed the problem for me.

Thanks!!!

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