Cancan 2.0 fix for issue #565; fixes namespaced non-db/model backed resources authorization #570

Merged
merged 2 commits into from May 11, 2012

Conversation

Projects
None yet
3 participants
@bsodmike

bsodmike commented Mar 2, 2012

This fixes the issue detailed in full here: #565 馃憦

@bsodmike

This comment has been minimized.

Show comment
Hide comment
@bsodmike

bsodmike Mar 30, 2012

Here's something interesting Ryan. I've returned working on an old app and until now I was treating its resource as a non-db backed resource; hence the patch above. However, now I'm doing CRUD as well.

class Admin::ScheduledSessionsController < AdminController
  include ApplicationHelper
  load_and_authorize_resource :class => "Session"

...and my abilities now look like:

class AdminAbility
  include CanCan::Ability
  def initialize(user)
    if user
      can [:index, :show], "admin/dashboard"
      can [:index, :show, :published, :unpublished], "admin/scheduled_sessions"

      if user.super_admin?
        can [:new, :create], "admin/scheduled_sessions"
        can [:new, :create], :sessions

        can [:edit, :update], "admin/scheduled_sessions"
        can [:edit, :update], :sessions do |s|
          s.applicant_signups.count == 0 
        end

        can [:publish, :unpublish], "admin/scheduled_sessions"
        can [:publish, :unpublish], :sessions

      end

    end
  end
end

Here's something interesting Ryan. I've returned working on an old app and until now I was treating its resource as a non-db backed resource; hence the patch above. However, now I'm doing CRUD as well.

class Admin::ScheduledSessionsController < AdminController
  include ApplicationHelper
  load_and_authorize_resource :class => "Session"

...and my abilities now look like:

class AdminAbility
  include CanCan::Ability
  def initialize(user)
    if user
      can [:index, :show], "admin/dashboard"
      can [:index, :show, :published, :unpublished], "admin/scheduled_sessions"

      if user.super_admin?
        can [:new, :create], "admin/scheduled_sessions"
        can [:new, :create], :sessions

        can [:edit, :update], "admin/scheduled_sessions"
        can [:edit, :update], :sessions do |s|
          s.applicant_signups.count == 0 
        end

        can [:publish, :unpublish], "admin/scheduled_sessions"
        can [:publish, :unpublish], :sessions

      end

    end
  end
end
@jeremyf

This comment has been minimized.

Show comment
Hide comment
@jeremyf

jeremyf May 11, 2012

Collaborator

@bsodmike Unfortunately, your patch does not merge cleanly against master. Could you rebase and submit again? Then ping me.

Collaborator

jeremyf commented May 11, 2012

@bsodmike Unfortunately, your patch does not merge cleanly against master. Could you rebase and submit again? Then ping me.

@bsodmike

This comment has been minimized.

Show comment
Hide comment
@bsodmike

bsodmike May 11, 2012

Hi @jeremyf done, can you try now please? Thanks!

Hi @jeremyf done, can you try now please? Thanks!

@jeremyf

This comment has been minimized.

Show comment
Hide comment
@jeremyf

jeremyf May 11, 2012

Collaborator

[Verified] Clean merge on 2.0; And the specs all pass.

Collaborator

jeremyf commented May 11, 2012

[Verified] Clean merge on 2.0; And the specs all pass.

@bsodmike

This comment has been minimized.

Show comment
Hide comment
@bsodmike

bsodmike May 11, 2012

Thanks - will this be merged into @ryanb's 2.0 branch soon?

Thanks - will this be merged into @ryanb's 2.0 branch soon?

@jeremyf

This comment has been minimized.

Show comment
Hide comment
@jeremyf

jeremyf May 11, 2012

Collaborator

@bsodmike While I have commit rights to the repo, I'm here to help triage things. Right now I'm trying to clear out the pull requests. As far as timing, Ryan's been merged several requests yesterday that I verified. So I assume so.

Collaborator

jeremyf commented May 11, 2012

@bsodmike While I have commit rights to the repo, I'm here to help triage things. Right now I'm trying to clear out the pull requests. As far as timing, Ryan's been merged several requests yesterday that I verified. So I assume so.

@bsodmike

This comment has been minimized.

Show comment
Hide comment
@bsodmike

bsodmike May 11, 2012

That's fine thanks. Yup, just noticed that @ryanb's been merging PR's in. Cheers for your efforts, really appreciated!

That's fine thanks. Yup, just noticed that @ryanb's been merging PR's in. Cheers for your efforts, really appreciated!

@ryanb

This comment has been minimized.

Show comment
Hide comment
@ryanb

ryanb May 11, 2012

Owner

I had been meaning to get something like this in, thank you for the pull request. Glad it is so easy as well.

Owner

ryanb commented May 11, 2012

I had been meaning to get something like this in, thank you for the pull request. Glad it is so easy as well.

ryanb added a commit that referenced this pull request May 11, 2012

Merge pull request #570 from bsodmike/bsodmike-2.0
Cancan 2.0 fix for issue #565; fixes namespaced non-db/model backed resources authorization

@ryanb ryanb merged commit 4986de8 into ryanb:2.0 May 11, 2012

@bsodmike

This comment has been minimized.

Show comment
Hide comment
@bsodmike

bsodmike May 11, 2012

Awesome @ryanb, my pleasure =)

Awesome @ryanb, my pleasure =)

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