DRY comments controller and add more spec scenarios

app/controllers/comments_controller.rb

@@ -1,17 +1,15 @@
 class CommentsController < ApplicationController
   before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
+  respond_to :html, :json
+  respond_to :rss, only: :index
 
   # GET /comments
   # GET /comments.json
+  # GET /comments.rss
   def index
     @comments = Comment.paginate(page: params[:page])
-
-    respond_to do |format|
-      format.html # index.html.erb
-      format.json { render json: @comments }
-      format.rss { render layout: false }
-    end
+    respond_with(@comments)
   end
 
   # GET /comments/new
@@ -22,13 +20,10 @@ class CommentsController < ApplicationController
 
     if @post
       @comments = @post.comments
-      respond_to do |format|
-        format.html # new.html.erb
-        format.json { render json: @comment }
-      end
+      respond_with(@comment)
     else
-      redirect_to request.referer || root_url,
-        alert: "Can't post a comment on a non-existent post"
+      redirect_to(request.referer || root_url,
+        alert: "Can't post a comment on a non-existent post")
     end
   end
 
@@ -40,37 +35,17 @@ class CommentsController < ApplicationController
   # POST /comments
   # POST /comments.json
   def create
-    params[:comment][:author_id] = current_member.id
     @comment = Comment.new(comment_params)
-
-    respond_to do |format|
-      if @comment.save
-        format.html { redirect_to @comment.post, notice: "Comment was successfully created." }
-        format.json { render json: @comment, status: :created, location: @comment }
-      else
-        format.html { render action: "new" }
-        format.json { render json: @comment.errors, status: :unprocessable_entity }
-      end
-    end
+    @comment.author = current_member
+    flash[:notice] = "Comment was successfully created." if @comment.save
+    respond_with(@comment.post)
   end
 
   # PUT /comments/1
   # PUT /comments/1.json
   def update
-    # you should never be able to change the author or post when
-    # updating
-    params[:comment].delete("post_id")
-    params[:comment].delete("author_id")
-
-    respond_to do |format|
-      if @comment.update(comment_params)
-        format.html { redirect_to @comment.post, notice: 'Comment was successfully updated.' }
-        format.json { head :no_content }
-      else
-        format.html { render action: "edit" }
-        format.json { render json: @comment.errors, status: :unprocessable_entity }
-      end
-    end
+    flash[:notice] = 'Comment was successfully updated.' if @comment.update(comment_params)
+    respond_with(@comment.post)
   end
 
   # DELETE /comments/1
@@ -78,16 +53,12 @@ class CommentsController < ApplicationController
   def destroy
     @post = @comment.post
     @comment.destroy
-
-    respond_to do |format|
-      format.html { redirect_to @post }
-      format.json { head :no_content }
-    end
+    respond_with(@post)
   end
 
   private
 
   def comment_params
-    params.require(:comment).permit(:author_id, :body, :post_id)
+    params.require(:comment).permit(:body, :post_id)
   end
 end

spec/controllers/comments_controller_spec.rb

@@ -21,7 +21,7 @@ describe CommentsController do
 
   def valid_attributes
     @post = FactoryGirl.create(:post)
-    { post_id: @post.id, author_id: @member.id, body: "some text" }
+    { post_id: @post.id, body: "some text" }
   end
 
   describe "GET RSS feed" do
@@ -34,51 +34,73 @@ describe CommentsController do
   end
 
   describe "GET new" do
-    it "picks up post from params" do
-      post = FactoryGirl.create(:post)
-      get :new, post_id: post.id
-      assigns(:post).should eq(post)
-    end
+    let(:post) { FactoryGirl.create(:post) }
 
-    it "assigns the old comments as @comments" do
-      post = FactoryGirl.create(:post)
-      old_comment = FactoryGirl.create(:comment, post: post)
-      get :new, post_id: post.id
-      assigns(:comments).should eq [old_comment]
+    describe "with valid params" do
+      before { get :new, post_id: post.id }
+
+      it "picks up post from params" do
+        assigns(:post).should eq(post)
+      end
+
+      let(:old_comment) { FactoryGirl.create(:comment, post: post) }
+      it "assigns the old comments as @comments" do
+        assigns(:comments).should eq [old_comment]
+      end
     end
 
     it "dies if no post specified" do
       get :new
-      response.should redirect_to(root_url)
+      expect(response).not_to be_success
     end
   end
 
   describe "GET edit" do
-    it "assigns previous comments as @comments" do
-      post = FactoryGirl.create(:post)
-      old_comment = FactoryGirl.create(:comment, post: post)
-      comment = FactoryGirl.create(:comment, post: post, author: @member)
-      get :edit, id: comment.to_param
-      assigns(:comments).should eq([comment, old_comment])
+    let(:post) { FactoryGirl.create(:post) }
+    before { get :edit, id: comment.to_param }
+
+    describe "my comment" do
+      let!(:comment) { FactoryGirl.create :comment, author: @member, post: post }
+      let!(:old_comment) { FactoryGirl.create(:comment, post: post, created_at: Time.zone.yesterday) }
+      it "assigns previous comments as @comments" do
+        assigns(:comments).should eq([comment, old_comment])
+      end
+    end
+
+    describe "not my comment" do
+      let(:comment) { FactoryGirl.create :comment, post: post }
+      it { expect(response).not_to be_success }
     end
   end
 
   describe "PUT update" do
-    describe "with valid params" do
+    before { put :update, id: comment.to_param, comment: valid_attributes }
+
+    describe "my comment" do
+      let(:comment) { FactoryGirl.create :comment, author: @member }
       it "redirects to the comment's post" do
-        comment = Comment.create! valid_attributes
-        put :update, id: comment.to_param, comment: valid_attributes
-        response.should redirect_to(comment.post)
+        expect(response).to redirect_to(comment.post)
       end
     end
+    describe "not my comment" do
+      let(:comment) { FactoryGirl.create :comment }
+      it { expect(response).not_to be_success }
+    end
   end
 
   describe "DELETE destroy" do
-    it "redirects to the post the comment was on" do
-      comment = Comment.create! valid_attributes
-      post = comment.post
-      delete :destroy, id: comment.to_param
-      response.should redirect_to(post)
+    before { delete :destroy, id: comment.to_param }
+
+    describe "my comment" do
+      let(:comment) { FactoryGirl.create :comment, author: @member }
+      it "redirects to the post the comment was on" do
+        expect(response).to redirect_to(comment.post)
+      end
+    end
+
+    describe "not my comment" do
+      let(:comment) { FactoryGirl.create :comment }
+      it { expect(response).not_to be_success }
     end
   end
 end