namespace problem after version 1.6.8 #663

Closed
diegorv opened this Issue Jun 26, 2012 · 18 comments

Projects

None yet

4 participants

@diegorv
diegorv commented Jun 26, 2012

After 1.6.8, things like that, stopped working

can :manage, CallCenter::Prospect

the error:

Expected /Users/diegorv/work/theERP/app/models/call_center/prospect.rb to define Prospect
@andhapp
Collaborator
andhapp commented Jun 26, 2012

@diegorv: Thanks for reporting the bug. Apart from this, did you see any thing else?

@diegorv
diegorv commented Jun 26, 2012

Not yet!

@andhapp
Collaborator
andhapp commented Jun 26, 2012

Cool. I'll look at this one. There were few pull requests related to namespace models (or controllers), which I guess might have introduced this regression.

@andhapp
Collaborator
andhapp commented Jun 26, 2012

@diegorv: Good news is that I could reproduce the error but to confirm that we have the same setup, just wanted to confirm one thing. You have the following model class:

class CallCenter::Prospect < ActiveRecord::Base
end

and this is in a file called app/models/prospect.rb. Please confirm.

Also, I'm guessing that when you upgraded CanCan, you also upgraded Rails version, because I could reproduce the same error with CanCan 1.6.7 and Rails 3.2.6, 3.2.5, 3.1.6, 3.1.5 and 3.1.4.

Just trying to establish where the bug is coming from and I suspect it's Rails. Please post your comments.

Update: Summarised my effort.

@andhapp andhapp was assigned Jun 26, 2012
@diegorv
diegorv commented Jun 27, 2012

Thank you for your work!

This is my complete info about my project

ruby 1.9.3p194 (2012-04-20 revision 35410) [x86_64-darwin11.3.0]
gem 'rails', '3.2.6'
gem 'cancan'

file: app/models/call_center/prospect.rb

class CallCenter::Prospect < ActiveRecord::Base
end

file: app/models/call_center.rb

module CallCenter
 def self.table_name_prefix
  'call_center_'
 end
end

file: app/models/users/ability.rb

class Ability
  include CanCan::Ability

  def initialize(user)
    @user = user || User.new
    @user.roles.each { |role| send(role.url) }
  end

  def telemarketing
    can :manage, CallCenter::Prospect
  end
end

Before problem:
rails 3.2.6
cancan 1.6.7

After problem
rails 3.2.6
cancan 1.6.8

no update in others gems

@andhapp
Collaborator
andhapp commented Jun 27, 2012

Thanks. I need to do more investigation, but I'm on the case.

@diegorv
diegorv commented Jun 27, 2012

I found the bug

in file: cancan/lib/cancan/controller_resource.rb, line 222

def namespace
  @params[:controller].split("::")[0..-2]
end

to debug, i changed it to:

def namespace
  puts "controller: #{@params[:controller]}"
  @params[:controller].split("::")[0..-2]
end

the output:

 controller: call_center/prospects

Change

@params[:controller].split("::")[0..-2]

To

@params[:controller].split("/")[0..-2]

Fixed this bug!

But not sure how that affects everything

@diegorv
diegorv commented Jun 27, 2012

Look this commit, this caused the bug

d5baed6 (9 months ago and merge NOW?! WTF?! too old guys)

Before this commit (working good)

def namespaced_name
 @name || @params[:controller].sub("Controller", "").singularize.camelize.constantize

 rescue NameError
   name
 end

After this commit (bug)

def namespace
  @params[:controller].split("::")[0..-2]
end

def namespaced_name
  [namespace, name.camelize].join('::').singularize.camelize.constantize
rescue NameError
  name
end

in older versions of Rails (like 9 months ago HAHAHAH), should work, but namespace is now "/" instead "::"

Regression for me.. :/

@andhapp
Collaborator
andhapp commented Jun 27, 2012

@diegorv: Nice work! Would you like to create a pull request for it please?

The commit that you pointed to was meant to fix the nested resource loading, so not sure why it caused this regression.

Like any other open source project, the main creator of this library @ryanb was busy with other assignments, I guess, and this kind of went on the back burner. On the positive side, I've been working through the pull requests and managing the issues.

Thanks again for finding the cause of this issue.

@diegorv
diegorv commented Jun 27, 2012

@andhapp I understand completely, so I'm trying to help too

I`ll make the pull request, but i don't have ruby 1.8.7, could u run the specs?

@andhapp
Collaborator
andhapp commented Jun 27, 2012

Okay. I will. Will close this issue as you've opened a related pull request. Thanks, again.

@andhapp andhapp closed this Jun 27, 2012
@andhapp
Collaborator
andhapp commented Jun 29, 2012

@diegorv : Hello, just looking at this again and I can't seem to replicate this issue anymore with the setup that you've mentioned in one of the comments above. So, I created a test application for the same and it'd be great if you could look at it and try replicating it at your end. Here's the repository (https://github.com/andhapp/cancan-issue-663).

This is what I do to test:

  1. Run the rails console
  2. Assuming there is a user in the database if not, please create one with User.create(username: 'test', password: 'test'
  3. In the console, do
    user = User.first # the user we just created
    ability = Ability.new(user) 
    ability.telemarketing # This should throw errors but for me it doesn't
    

Please test and let us know the outcome.

@andhapp andhapp reopened this Jun 29, 2012
@diegorv
diegorv commented Jun 29, 2012

@andhapp, your example is wrong, because without user role "telemarketing" like that

def roles
  ['telemarketing']
end

This

def initialize(user)
  @user = user || User.new
  @user.roles.each { |role| send(role) }
end

Will never call

def telemarketing
  can :manage, CallCenter::Prospect
end

Take a look in my fork: https://github.com/diegorv/cancan-issue-663, read "README"

The same problem, with different fix in #668 (I prefer this instead of my)

@andhapp
Collaborator
andhapp commented Jun 29, 2012

@diegrov: Ok gotcha. That pull request is against 2.x branch...so we need backport it for 1.x branch. I'll close this issue and close your pull request and then port the #668 to 1.x branch. Thanks for taking the time to look at my app.

@andhapp andhapp closed this Jun 29, 2012
@diegorv
diegorv commented Jun 29, 2012

@andhapp you're welcome! keep rocking :)

@softpro
softpro commented Mar 1, 2013

WTF?! The bug is still present in last version I can see (1.6.9) and in the master branch: https://github.com/ryanb/cancan/blob/master/lib/cancan/controller_resource.rb. Does anybody care?

@diegorv
diegorv commented Mar 1, 2013

@andhapp ?
@ryanb ?

:/

@jaredbeck

This issue has been fixed in 1.6.10 by #675 (60cf6a6)

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