Skip to content
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

Redirect to root_path unless admin_root_path #2821

Closed
wants to merge 1 commit into from

Conversation

developer88
Copy link

If just a user tries to navigate to admin#index it receives 'Cannot redirect to nil!'. I propose to redirect to root path.

If just a user tries to navigate to admin#index it receives 'Cannot redirect to nil!'. I propose to redirect to root path.
@simi
Copy link
Member

simi commented Dec 29, 2014

Can you describe me usecase? I don't understand this change.

@parndt
Copy link
Member

parndt commented Dec 30, 2014

At the very least, we need a test demonstrating the use case that fails with the old code too.

@developer88
Copy link
Author

@parndt I will add some specs a bit later
@simi What happened if user that is not an admin navigate to admin_root_path? In my case this method just returns nil and than i receives 500 error (because redirect_to nil).

So. i have added a refinery.root_path to redirect user back to root page of CMS

@parndt
Copy link
Member

parndt commented Jan 4, 2015

I have a hesitation based around the implementation. Currently the method is called refinery_admin_root_path but it is, in fact, returning refinery.root_path under the case where they're not logged in. This is kind of confusing!

Furthermore, I'm confused as to why this method is even being accessed when the user is not logged in, as this is an admin method. Perhaps we have another design error (i.e. we're not restricting early enough) that this would simply be covering up?

@parndt
Copy link
Member

parndt commented Jan 23, 2015

I believe that if we are to patch this, @developer88, it should be here: https://github.com/refinery/refinerycms/blob/master/core/app/controllers/refinery/admin/core_controller.rb#L5

module Refinery
  module Admin
    class CoreController < ::Refinery::AdminController
      def index
        redirect_to(refinery_admin_root_path || refinery_root_path)
      end

@parndt parndt closed this Feb 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants