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

Switch auth system to cancan WIP #69

Merged
merged 31 commits into from
Mar 26, 2015
Merged

Switch auth system to cancan WIP #69

merged 31 commits into from
Mar 26, 2015

Conversation

henrikssn
Copy link
Contributor

This PR will migrate the code base from TheRole to cancancan.

Solves #67 and needs #68

Note: Work in progress, help is very welcome!

class Ability
include CanCan::Ability

def initialize(user)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused method argument - user. If it's necessary, use _ or _user as an argument name to indicate that it won't be used. You can also write as initialize(*) if you want the method to accept any arguments but don't care about them.

@henrikssn henrikssn mentioned this pull request Mar 19, 2015
@davidwessman
Copy link
Member

How will we give a user permissions? Can it be done depending on their Post?
Or what did you have in mind? @henrikssn @jforberg @lham

@henrikssn
Copy link
Contributor Author

@Zofz My proposal:

model Permission
  has_and_belongs_to_many :posts
  :subject_class # model names like User, Role, Book, Author
  :action        # controller action like new, create or destroy
model Post
  has_and_belongs_to_many :permissions

Then do something like
http://blog.joshsoftware.com/2012/10/23/dynamic-roles-and-permissions-using-cancan/

@jforberg
Copy link
Contributor

Jag gillar @henrikssn lösning. Bättre att jobba på normalformen än att knacka in JSON-strängar i sin databas. JSONkolumner luktar lite som att man inte riktigt har koll på hur databaser funkar.

Dock skulle jag inte använda HABTM. Jag vet att det lärs ut fortfarande men det är inte bra av två skäl:

  1. HABTM stödjer inte valideringar, därför att
  2. HABTM är databasinkontinent, den committar alltid omedelbart. Vilket oftast inte är vad man vill

has_many :through => är ett bättre alternativ i de flesta fall alltid.

@davidwessman
Copy link
Member

Implementing more tests in #87

@@ -61,7 +61,7 @@ def candidates
end
private
def authenticate
flash[:error] = t('the_role.access_denied')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace detected.

protect_from_forgery

before_filter :configure_permitted_devise_parameters, if: :devise_controller?
before_filter :set_locale
before_filter :get_commit

rescue_from CanCan::AccessDenied do |ex|
flash[:error] = ex.message
render :text => '', :layout => true, :status => :forbidden

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the new Ruby 1.9 hash syntax.

Conflicts:
	Gemfile
	Gemfile.lock
	app/controllers/application_controller.rb
	app/controllers/events_controller.rb
	app/controllers/posts_controller.rb
	app/controllers/static_pages_controller.rb
	app/views/layouts/_topbar.html.erb
	app/views/layouts/_utskottmenu.html.erb
	app/views/posts/_post.html.erb
	config/routes.rb
	db/schema.rb
@@ -101,12 +100,24 @@ def destroy

private
def authenticate
<<<<<<< HEAD

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unexpected token tLSHFT

before_action :set_post, only: [:show, :edit, :update, :destroy, :remove_profile,:add_profile_username, :display]

def remove_profile
@profile = Profile.find_by_id(params[:profile_id])
@post.profiles.delete(@profile)
respond_to do |format|
format.html { redirect_to council_posts_path(@council), notice: @profile.name.to_s + ' har inte längre posten ' + @post.title.to_s + '.'}
format.html { redirect_to council_posts_path(@council), notice: @profile.name.to_s + ' har inte längre posten ' + @post.title.to_s + '.'}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [143/100]
Space missing inside }.

@henrikssn
Copy link
Contributor Author

@fsek/admins Will merge this later today or tomorrow. Please take a look.

@jforberg
Copy link
Contributor

Looks like great work @henrikssn

rescue ActionController::RedirectBackError
redirect_to root_path

def set_councils

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation detected.

@henrikssn
Copy link
Contributor Author

Merging when test results arrive...

henrikssn added a commit that referenced this pull request Mar 26, 2015
Switch auth system to cancan WIP
@henrikssn henrikssn merged commit 86e473e into master Mar 26, 2015
@henrikssn
Copy link
Contributor Author

Fingers crossed 👌

@ghost
Copy link

ghost commented Mar 26, 2015

This PR was deployed to Production. Reference: 86e473e

@davidwessman
Copy link
Member

👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants