#465: Update specs for Projects::CommentsController

app/controllers/projects/comments_controller.rb

@@ -1,26 +1,20 @@
 class Projects::CommentsController < Projects::BaseController
   before_action :authenticate_user!
-  load_and_authorize_resource :project
   before_action :find_commentable
   before_action :find_or_build_comment
-  load_and_authorize_resource new: :new_line
 
   include CommentsHelper
 
   def create
-    respond_to do |format|
+    if !@comment.set_additional_data params
-      if !@comment.set_additional_data params
+      render json: {
-        format.json {
+        error:   I18n.t("flash.comment.save_error"),
-                      render json: {
+        message: @comment.errors.full_messages
-                                     error:   I18n.t("flash.comment.save_error"),
+      }
-                                     message: @comment.errors.full_messages
+    elsif @comment.save
-                                   }
+      render :create
-                    }
+    else
-      elsif @comment.save
+      render json: { error: I18n.t("flash.comment.save_error") }, status: 422
-        format.json {}
-      else
-        format.json { render json: { error: I18n.t("flash.comment.save_error") }, status: 422 }
-      end
     end
   end
 
@@ -51,5 +45,6 @@ class Projects::CommentsController < Projects::BaseController
   def find_or_build_comment
     @comment = params[:id].present? && Comment.where(automatic: false).find(params[:id]) ||
                current_user.comments.build(params[:comment]) {|c| c.commentable = @commentable; c.project = @project}
+    authorize @comment
   end
 end

app/policies/comment_policy.rb

@@ -6,7 +6,8 @@ class CommentPolicy < ApplicationPolicy
   alias_method :new_line?, :create?
 
   def update?
-    record.user_id == user.id || local_admin?(record.project)
+    is_admin? || record.user_id == user.id || local_admin?(record.project)
   end
+  alias_method :destroy?, :update?
 
 end

spec/support/shared_examples/comments.rb

@@ -38,63 +38,69 @@ end
 shared_examples_for 'user with create comment ability' do
   it 'should be able to perform create action' do
     post :create, @create_params
-    response.should be_success #redirect_to(@return_path+"#comment#{Comment.last.id}")
+    expect(response).to be_success #redirect_to(@return_path+"#comment#{Comment.last.id}")
   end
 
   it 'should create comment in the database' do
-    lambda{ post :create, @create_params }.should change{ Comment.count }.by(1)
+    expect do
+      post :create, @create_params
+    end.to change(Comment, :count).by(1)
   end
 end
 shared_examples_for 'user with update own comment ability' do
   it 'should be able to perform update action' do
     put :update, {id: @own_comment.id}.merge(@update_params)
-    response.status.should == 200
+    expect(response).to be_success
   end
 
   it 'should update subscribe body' do
     put :update, {id: @own_comment.id}.merge(@update_params)
-    @own_comment.reload.body.should == 'updated'
+    expect(@own_comment.reload.body).to eq 'updated'
   end
 end
 shared_examples_for 'user with update stranger comment ability' do
   it 'should be able to perform update action' do
     put :update, {id: @comment.id}.merge(@update_params)
-    response.status.should == 200
+    expect(response).to be_success
   end
 
   it 'should update comment body' do
     put :update, {id: @comment.id}.merge(@update_params)
-    @comment.reload.body.should == 'updated'
+    expect(@comment.reload.body).to eq 'updated'
   end
 end
 shared_examples_for 'user without update stranger comment ability' do
   it 'should not be able to perform update action' do
     put :update, {id: @comment.id}.merge(@update_params)
-    response.should redirect_to(forbidden_path)
+    expect(response).to redirect_to(forbidden_path)
   end
 
   it 'should not update comment body' do
     put :update, {id: @comment.id}.merge(@update_params)
-    @comment.reload.body.should_not == 'updated'
+    expect(@comment.reload.body).to_not eq 'updated'
   end
 end
 shared_examples_for 'user with destroy comment ability' do
   it 'should be able to perform destroy action' do
     delete :destroy, {id: @comment.id}.merge(@path)
-    response.should be_success #redirect_to(@return_path)
+    expect(response).to be_success #redirect_to(@return_path)
   end
 
   it 'should delete comment from database' do
-    lambda{ delete :destroy, {id: @comment.id}.merge(@path)}.should change{ Comment.count }.by(-1)
+    expect do
+      delete :destroy, {id: @comment.id}.merge(@path)
+    end.to change(Comment, :count).by(-1)
   end
 end
 shared_examples_for 'user without destroy comment ability' do
   it 'should not be able to perform destroy action' do
     delete :destroy, {id: @comment.id}.merge(@path)
-    response.should redirect_to(forbidden_path)
+    expect(response).to redirect_to(forbidden_path)
   end
 
   it 'should not delete comment from database' do
-    lambda{ delete :destroy, {id: @comment.id}.merge(@path)}.should change{ Issue.count }.by(0)
+    expect do
+      delete :destroy, {id: @comment.id}.merge(@path)
+    end.to_not change(Issue, :count)
   end
 end