From 5155cbc247be6649bb553b3cf661b8f435aaabb3 Mon Sep 17 00:00:00 2001 From: Vasiliy Ermolovich Date: Mon, 4 Jun 2012 21:50:09 +0300 Subject: [PATCH] extract comments logic to CommentsController --- app/controllers/comments_controller.rb | 43 +++++++++++++++++++++++++++++++++++++++++++ app/controllers/errs_controller.rb | 23 ----------------------- app/views/errs/show.html.haml | 4 ++-- config/routes.rb | 4 ++-- spec/controllers/comments_controller_spec.rb | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ spec/controllers/errs_controller_spec.rb | 54 ------------------------------------------------------ 6 files changed, 106 insertions(+), 81 deletions(-) create mode 100644 app/controllers/comments_controller.rb create mode 100644 spec/controllers/comments_controller_spec.rb diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb new file mode 100644 index 0000000..c0e7160 --- /dev/null +++ b/app/controllers/comments_controller.rb @@ -0,0 +1,43 @@ +class CommentsController < ApplicationController + before_filter :find_app + before_filter :find_problem + + def create + @comment = Comment.new(params[:comment].merge(:user_id => current_user.id)) + if @comment.valid? + @problem.comments << @comment + @problem.save + flash[:success] = "Comment saved!" + else + flash[:error] = "I'm sorry, your comment was blank! Try again?" + end + redirect_to app_err_path(@app, @problem) + end + + def destroy + @comment = Comment.find(params[:id]) + if @comment.destroy + flash[:success] = "Comment deleted!" + else + flash[:error] = "Sorry, I couldn't delete your comment for some reason. I hope you don't have any sensitive information in there!" + end + redirect_to app_err_path(@app, @problem) + end + + protected + def find_app + @app = App.find(params[:app_id]) + + # Mongoid Bug: could not chain: current_user.apps.find_by_id! + # apparently finding by 'watchers.email' and 'id' is broken + raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app) + end + + def find_problem + @problem = @app.problems.find(params[:err_id]) + + # Deal with bug in mogoid where find is returning an Enumberable obj + @problem = @problem.first if @problem.respond_to?(:first) + end +end + diff --git a/app/controllers/errs_controller.rb b/app/controllers/errs_controller.rb index 8ec08fc..bcbf6c4 100644 --- a/app/controllers/errs_controller.rb +++ b/app/controllers/errs_controller.rb @@ -88,29 +88,6 @@ class ErrsController < ApplicationController redirect_to app_path(@app) end - def create_comment - @comment = Comment.new(params[:comment].merge(:user_id => current_user.id)) - if @comment.valid? - @problem.comments << @comment - @problem.save - flash[:success] = "Comment saved!" - else - flash[:error] = "I'm sorry, your comment was blank! Try again?" - end - redirect_to app_err_path(@app, @problem) - end - - def destroy_comment - @comment = Comment.find(params[:comment_id]) - if @comment.destroy - flash[:success] = "Comment deleted!" - else - flash[:error] = "Sorry, I couldn't delete your comment for some reason. I hope you don't have any sensitive information in there!" - end - redirect_to app_err_path(@app, @problem) - end - - def resolve_several @selected_problems.each(&:resolve!) flash[:success] = "Great news everyone! #{pluralize(@selected_problems.count, 'err has', 'errs have')} been resolved." diff --git a/app/views/errs/show.html.haml b/app/views/errs/show.html.haml index 0917d31..0e7de08 100644 --- a/app/views/errs/show.html.haml +++ b/app/views/errs/show.html.haml @@ -27,13 +27,13 @@ %table.comment %tr %th - %span= link_to '✘'.html_safe, destroy_comment_app_err_path(@app, @problem) << "?comment_id=#{comment.id}", :method => :delete, :confirm => "Are sure you don't need this comment?", :class => "destroy-comment" + %span= link_to '✘'.html_safe, app_err_comment_path(@app, @problem, comment), :method => :delete, :confirm => "Are sure you don't need this comment?", :class => "destroy-comment" = time_ago_in_words(comment.created_at, true) << " ago by " = link_to comment.user.email, user_path(comment.user) %tr %td= comment.body.gsub("\n", "
").html_safe - if Errbit::Config.allow_comments_with_issue_tracker || !@app.issue_tracker_configured? - = form_for @comment, :url => create_comment_app_err_path(@app, @problem) do |comment_form| + = form_for @comment, :url => app_err_comments_path(@app, @problem) do |comment_form| %p Add a comment = comment_form.text_area :body, :style => "width: 420px; height: 80px;" = comment_form.submit "Save Comment" diff --git a/config/routes.rb b/config/routes.rb index 8adf1e9..37667b2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,13 +27,13 @@ Errbit::Application.routes.draw do resources :apps do resources :errs do resources :notices + resources :comments, :only => [:create, :destroy] + member do put :resolve put :unresolve post :create_issue delete :unlink_issue - post :create_comment - delete :destroy_comment end end diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb new file mode 100644 index 0000000..12a6cee --- /dev/null +++ b/spec/controllers/comments_controller_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe CommentsController do + let(:app) { Fabricate(:app) } + let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :environment => "production")) } + + describe "POST /apps/:app_id/errs/:id/comments/create" do + render_views + + before(:each) do + sign_in Fabricate(:admin) + end + + context "successful comment creation" do + let(:problem) { Fabricate(:problem) } + let(:user) { Fabricate(:user) } + + before(:each) do + post :create, :app_id => problem.app.id, :err_id => problem.id, + :comment => { :body => "One test comment", :user_id => user.id } + problem.reload + end + + it "should create the comment" do + problem.comments.size.should == 1 + end + + it "should redirect to problem page" do + response.should redirect_to( app_err_path(problem.app, problem) ) + end + end + end + + describe "DELETE /apps/:app_id/errs/:id/comments/:id/destroy" do + render_views + + before(:each) do + sign_in Fabricate(:admin) + end + + context "successful comment deletion" do + let(:problem) { Fabricate(:problem_with_comments) } + let(:comment) { problem.reload.comments.first } + + before(:each) do + delete :destroy, :app_id => problem.app.id, :err_id => problem.id, :id => comment.id.to_s + problem.reload + end + + it "should delete the comment" do + problem.comments.detect{|c| c.id.to_s == comment.id }.should == nil + end + + it "should redirect to problem page" do + response.should redirect_to( app_err_path(problem.app, problem) ) + end + end + end +end diff --git a/spec/controllers/errs_controller_spec.rb b/spec/controllers/errs_controller_spec.rb index 61dfe5c..33a8136 100644 --- a/spec/controllers/errs_controller_spec.rb +++ b/spec/controllers/errs_controller_spec.rb @@ -372,60 +372,6 @@ describe ErrsController do end end - - describe "POST /apps/:app_id/errs/:id/create_comment" do - render_views - - before(:each) do - sign_in Fabricate(:admin) - end - - context "successful comment creation" do - let(:problem) { Fabricate(:problem) } - let(:user) { Fabricate(:user) } - - before(:each) do - post :create_comment, :app_id => problem.app.id, :id => problem.id, - :comment => { :body => "One test comment", :user_id => user.id } - problem.reload - end - - it "should create the comment" do - problem.comments.size.should == 1 - end - - it "should redirect to problem page" do - response.should redirect_to( app_err_path(problem.app, problem) ) - end - end - end - - describe "DELETE /apps/:app_id/errs/:id/destroy_comment" do - render_views - - before(:each) do - sign_in Fabricate(:admin) - end - - context "successful comment deletion" do - let(:problem) { Fabricate(:problem_with_comments) } - let(:comment) { problem.reload.comments.first } - - before(:each) do - delete :destroy_comment, :app_id => problem.app.id, :id => problem.id, :comment_id => comment.id.to_s - problem.reload - end - - it "should delete the comment" do - problem.comments.detect{|c| c.id.to_s == comment.id }.should == nil - end - - it "should redirect to problem page" do - response.should redirect_to( app_err_path(problem.app, problem) ) - end - end - end - describe "Bulk Actions" do before(:each) do sign_in Fabricate(:admin) -- libgit2 0.21.2