From 493ec76a1afc2787d7a80f02026dc2ddc32d5c5e Mon Sep 17 00:00:00 2001 From: Julien Letessier Date: Mon, 26 Aug 2013 16:12:24 +0100 Subject: [PATCH 1/4] Redirects using 303 on destroy. Fixes #142. --- app/controllers/votes_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/votes_controller.rb b/app/controllers/votes_controller.rb index 7338983..3002ca0 100644 --- a/app/controllers/votes_controller.rb +++ b/app/controllers/votes_controller.rb @@ -29,7 +29,7 @@ def destroy respond_to do |format| format.html { redirect_to @return_to, :notice => votes_message(@vote, :cancel) } - format.js { redirect_to @subject } + format.js { redirect_to @subject, status:303 } end end From 41c2114773e470c9bd5fbc41478fbf00f2e0e2e3 Mon Sep 17 00:00:00 2001 From: Julien Letessier Date: Mon, 26 Aug 2013 16:12:37 +0100 Subject: [PATCH 2/4] Adds authorization to votes/comments. --- app/controllers/comments_controller.rb | 6 ++++++ app/controllers/votes_controller.rb | 2 ++ 2 files changed, 8 insertions(+) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 5804da8..00897d4 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -7,6 +7,7 @@ class CommentsController < ApplicationController def create @comment = Comment.new(params[:comment]) @comment.author = current_user + authorize! :create, @comment success = @comment.save @@ -30,6 +31,8 @@ def create def show + authorize! :read, @comment + if request.xhr? case params['part'] when 'attachments' @@ -46,6 +49,8 @@ def show def update + authorize! :update, @comment + if @comment.update_attributes(params[:comment]) flash[:success] = _("Successfully updated comment.") else @@ -56,6 +61,7 @@ def update def destroy + authorize! :destroy, @comment @comment.destroy respond_to do |format| diff --git a/app/controllers/votes_controller.rb b/app/controllers/votes_controller.rb index 3002ca0..d89fc5f 100644 --- a/app/controllers/votes_controller.rb +++ b/app/controllers/votes_controller.rb @@ -10,6 +10,7 @@ class VotesController < ApplicationController def create @vote = @subject.votes.new(params[:vote]) @vote.user = current_user + authorize! :create, @vote if @vote.save flash[:success] = votes_message(@vote, :ok) @@ -26,6 +27,7 @@ def create def destroy @vote = @subject.votes.find(params[:id]) @vote.destroy + authorize! :destroy, @vote respond_to do |format| format.html { redirect_to @return_to, :notice => votes_message(@vote, :cancel) } From ed4af06a76a946dae63123b7341bf63ff29895dc Mon Sep 17 00:00:00 2001 From: Julien Letessier Date: Mon, 26 Aug 2013 22:19:17 +0100 Subject: [PATCH 3/4] Fixes authorisation in votes controller --- app/controllers/comments_controller.rb | 2 +- app/controllers/votes_controller.rb | 2 +- app/models/ability.rb | 9 ++-- spec/controllers/votes_controller_spec.rb | 58 ++++++++++++++++------- 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 00897d4..7e5f5d0 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -7,7 +7,7 @@ class CommentsController < ApplicationController def create @comment = Comment.new(params[:comment]) @comment.author = current_user - authorize! :create, @comment + authorize! :create, @comment if @comment.valid? success = @comment.save diff --git a/app/controllers/votes_controller.rb b/app/controllers/votes_controller.rb index d89fc5f..0811377 100644 --- a/app/controllers/votes_controller.rb +++ b/app/controllers/votes_controller.rb @@ -10,7 +10,7 @@ class VotesController < ApplicationController def create @vote = @subject.votes.new(params[:vote]) @vote.user = current_user - authorize! :create, @vote + authorize! :vote, @subject if @vote.valid? if @vote.save flash[:success] = votes_message(@vote, :ok) diff --git a/app/models/ability.rb b/app/models/ability.rb index c5997ee..0232e8e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -7,15 +7,19 @@ def initialize(user) # Idea permissions draft_or_submitted = [:draft, :submitted].map { |sym| Idea.state_value(sym) } - can :read, Idea, author: { account_id: user.account_id } + can :read, Idea, account_id: user.account_id if user.plays?(:submitter) - can :create, Idea, author: { account_id: user.account_id } + can :create, Idea, account_id: user.account_id can :update, Idea do |idea| idea.author == user && [:draft, :submitted].include?(idea.state_name) end can :destroy, Idea, author_id: user.id, state: draft_or_submitted end + can :vote, Idea do |idea| + user.account_id == idea.account_id + end + can :move, Idea do |idea| user == idea.author || user == idea.product_manager || user.plays?(:account_owner) end @@ -47,7 +51,6 @@ def initialize(user) end # Vote - can :create, Vote can :destroy, Vote do |r| r.user_id == user.id && r.recently_created? end diff --git a/spec/controllers/votes_controller_spec.rb b/spec/controllers/votes_controller_spec.rb index 6bad4a3..cc55be5 100644 --- a/spec/controllers/votes_controller_spec.rb +++ b/spec/controllers/votes_controller_spec.rb @@ -10,30 +10,54 @@ context '(vote on ideas)' do let(:idea) { ideas(:idea_submitted) } let(:vote) { Vote.make! subject:idea } - let(:comment) { Comment.make! } - it "create action should render new template when model is invalid" do - Vote.any_instance.stub(:valid? => false) - post :create, :idea_id => idea.id - response.should redirect_to(idea_path(idea)) + describe '#create' do + it "redirects to idea when model is invalid" do + Vote.any_instance.stub(:valid? => false) + post :create, :idea_id => idea.id + response.should redirect_to(idea_path(idea)) + end end - it "destroy action should destroy model and redirect to index action" do - delete :destroy, :id => vote, :idea_id => idea.id - response.should redirect_to(idea_path(idea)) - Vote.exists?(vote.id).should be_false + describe '#destroy' do + it "destroys vote" do + delete :destroy, :id => vote, :idea_id => idea.id + Vote.exists?(vote.id).should be_false + end + + it "redirects to idea" do + delete :destroy, :id => vote, :idea_id => idea.id + response.should redirect_to(idea_path(idea)) + end end + end - it "works on comments" do - lambda { + context '(vote on comments)' do + let(:idea) { ideas(:idea_submitted) } + let(:comment) { Comment.make!(author:User.make!) } + + describe '#create' do + it "redirects to the idea" do post :create, :comment_id => comment.id - }.should change { comment.votes.count }.by(1) - end + response.should redirect_to(idea_path(comment.idea)) + end - it "parses voting direction" do - post :create, :comment_id => comment.id, vote: { up: 'false' } - Vote.last.up.should be_false - end + it "creates a vote" do + lambda { + post :create, :comment_id => comment.id + }.should change { comment.votes.count }.by(1) + end + + it "prevents self-votes" do + comment.update_column :author_id, @current_user.id + post :create, :comment_id => comment.id + response.should be_forbidden + end + it "parses voting direction" do + post :create, :comment_id => comment.id, vote: { up: 'false' } + Vote.last.up.should be_false + end + end end end From af215040e20acc6ab2d1056ab66aa7a03379fef7 Mon Sep 17 00:00:00 2001 From: Julien Letessier Date: Tue, 27 Aug 2013 00:10:59 +0100 Subject: [PATCH 4/4] Fixes broken cuke --- app/views/ideas/_action.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/ideas/_action.html.haml b/app/views/ideas/_action.html.haml index 4790b3b..40a83f3 100644 --- a/app/views/ideas/_action.html.haml +++ b/app/views/ideas/_action.html.haml @@ -57,7 +57,7 @@ / vote - if idea.can_vote»? - - if can?(:create, idea.votes.build) + - if can?(:vote, idea) - if vote = current_user.votes.on_idea(idea).first %li = link_to idea_vote_path(idea, vote), method: :delete, class: 'btn btn-block', title: s_('Tooltip|By pressing this you cancel your endorsement of this story.
You will get back %{points} %{karma_icon}.
Make sure you add a comment to explain why you canceled!') % { points: -§.karma.vote, karma_icon: user_karma_symbol }, :'data-placement' => placement do