Commit ba2315ce2680488c55e2e3b2ced614e816e7e84a
Exists in
master
and in
1 other branch
Merge pull request #195 from errbit/comments-controller
extract comments logic to CommentsController
Showing
6 changed files
with
106 additions
and
81 deletions
Show diff stats
| ... | ... | @@ -0,0 +1,43 @@ |
| 1 | +class CommentsController < ApplicationController | |
| 2 | + before_filter :find_app | |
| 3 | + before_filter :find_problem | |
| 4 | + | |
| 5 | + def create | |
| 6 | + @comment = Comment.new(params[:comment].merge(:user_id => current_user.id)) | |
| 7 | + if @comment.valid? | |
| 8 | + @problem.comments << @comment | |
| 9 | + @problem.save | |
| 10 | + flash[:success] = "Comment saved!" | |
| 11 | + else | |
| 12 | + flash[:error] = "I'm sorry, your comment was blank! Try again?" | |
| 13 | + end | |
| 14 | + redirect_to app_err_path(@app, @problem) | |
| 15 | + end | |
| 16 | + | |
| 17 | + def destroy | |
| 18 | + @comment = Comment.find(params[:id]) | |
| 19 | + if @comment.destroy | |
| 20 | + flash[:success] = "Comment deleted!" | |
| 21 | + else | |
| 22 | + flash[:error] = "Sorry, I couldn't delete your comment for some reason. I hope you don't have any sensitive information in there!" | |
| 23 | + end | |
| 24 | + redirect_to app_err_path(@app, @problem) | |
| 25 | + end | |
| 26 | + | |
| 27 | + protected | |
| 28 | + def find_app | |
| 29 | + @app = App.find(params[:app_id]) | |
| 30 | + | |
| 31 | + # Mongoid Bug: could not chain: current_user.apps.find_by_id! | |
| 32 | + # apparently finding by 'watchers.email' and 'id' is broken | |
| 33 | + raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app) | |
| 34 | + end | |
| 35 | + | |
| 36 | + def find_problem | |
| 37 | + @problem = @app.problems.find(params[:err_id]) | |
| 38 | + | |
| 39 | + # Deal with bug in mogoid where find is returning an Enumberable obj | |
| 40 | + @problem = @problem.first if @problem.respond_to?(:first) | |
| 41 | + end | |
| 42 | +end | |
| 43 | + | ... | ... |
app/controllers/errs_controller.rb
| ... | ... | @@ -88,29 +88,6 @@ class ErrsController < ApplicationController |
| 88 | 88 | redirect_to app_path(@app) |
| 89 | 89 | end |
| 90 | 90 | |
| 91 | - def create_comment | |
| 92 | - @comment = Comment.new(params[:comment].merge(:user_id => current_user.id)) | |
| 93 | - if @comment.valid? | |
| 94 | - @problem.comments << @comment | |
| 95 | - @problem.save | |
| 96 | - flash[:success] = "Comment saved!" | |
| 97 | - else | |
| 98 | - flash[:error] = "I'm sorry, your comment was blank! Try again?" | |
| 99 | - end | |
| 100 | - redirect_to app_err_path(@app, @problem) | |
| 101 | - end | |
| 102 | - | |
| 103 | - def destroy_comment | |
| 104 | - @comment = Comment.find(params[:comment_id]) | |
| 105 | - if @comment.destroy | |
| 106 | - flash[:success] = "Comment deleted!" | |
| 107 | - else | |
| 108 | - flash[:error] = "Sorry, I couldn't delete your comment for some reason. I hope you don't have any sensitive information in there!" | |
| 109 | - end | |
| 110 | - redirect_to app_err_path(@app, @problem) | |
| 111 | - end | |
| 112 | - | |
| 113 | - | |
| 114 | 91 | def resolve_several |
| 115 | 92 | @selected_problems.each(&:resolve!) |
| 116 | 93 | flash[:success] = "Great news everyone! #{pluralize(@selected_problems.count, 'err has', 'errs have')} been resolved." | ... | ... |
app/views/errs/show.html.haml
| ... | ... | @@ -27,13 +27,13 @@ |
| 27 | 27 | %table.comment |
| 28 | 28 | %tr |
| 29 | 29 | %th |
| 30 | - %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" | |
| 30 | + %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" | |
| 31 | 31 | = time_ago_in_words(comment.created_at, true) << " ago by " |
| 32 | 32 | = link_to comment.user.email, user_path(comment.user) |
| 33 | 33 | %tr |
| 34 | 34 | %td= comment.body.gsub("\n", "<br>").html_safe |
| 35 | 35 | - if Errbit::Config.allow_comments_with_issue_tracker || !@app.issue_tracker_configured? |
| 36 | - = form_for @comment, :url => create_comment_app_err_path(@app, @problem) do |comment_form| | |
| 36 | + = form_for @comment, :url => app_err_comments_path(@app, @problem) do |comment_form| | |
| 37 | 37 | %p Add a comment |
| 38 | 38 | = comment_form.text_area :body, :style => "width: 420px; height: 80px;" |
| 39 | 39 | = comment_form.submit "Save Comment" | ... | ... |
config/routes.rb
| ... | ... | @@ -27,13 +27,13 @@ Errbit::Application.routes.draw do |
| 27 | 27 | resources :apps do |
| 28 | 28 | resources :errs do |
| 29 | 29 | resources :notices |
| 30 | + resources :comments, :only => [:create, :destroy] | |
| 31 | + | |
| 30 | 32 | member do |
| 31 | 33 | put :resolve |
| 32 | 34 | put :unresolve |
| 33 | 35 | post :create_issue |
| 34 | 36 | delete :unlink_issue |
| 35 | - post :create_comment | |
| 36 | - delete :destroy_comment | |
| 37 | 37 | end |
| 38 | 38 | end |
| 39 | 39 | ... | ... |
| ... | ... | @@ -0,0 +1,59 @@ |
| 1 | +require 'spec_helper' | |
| 2 | + | |
| 3 | +describe CommentsController do | |
| 4 | + let(:app) { Fabricate(:app) } | |
| 5 | + let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :environment => "production")) } | |
| 6 | + | |
| 7 | + describe "POST /apps/:app_id/errs/:id/comments/create" do | |
| 8 | + render_views | |
| 9 | + | |
| 10 | + before(:each) do | |
| 11 | + sign_in Fabricate(:admin) | |
| 12 | + end | |
| 13 | + | |
| 14 | + context "successful comment creation" do | |
| 15 | + let(:problem) { Fabricate(:problem) } | |
| 16 | + let(:user) { Fabricate(:user) } | |
| 17 | + | |
| 18 | + before(:each) do | |
| 19 | + post :create, :app_id => problem.app.id, :err_id => problem.id, | |
| 20 | + :comment => { :body => "One test comment", :user_id => user.id } | |
| 21 | + problem.reload | |
| 22 | + end | |
| 23 | + | |
| 24 | + it "should create the comment" do | |
| 25 | + problem.comments.size.should == 1 | |
| 26 | + end | |
| 27 | + | |
| 28 | + it "should redirect to problem page" do | |
| 29 | + response.should redirect_to( app_err_path(problem.app, problem) ) | |
| 30 | + end | |
| 31 | + end | |
| 32 | + end | |
| 33 | + | |
| 34 | + describe "DELETE /apps/:app_id/errs/:id/comments/:id/destroy" do | |
| 35 | + render_views | |
| 36 | + | |
| 37 | + before(:each) do | |
| 38 | + sign_in Fabricate(:admin) | |
| 39 | + end | |
| 40 | + | |
| 41 | + context "successful comment deletion" do | |
| 42 | + let(:problem) { Fabricate(:problem_with_comments) } | |
| 43 | + let(:comment) { problem.reload.comments.first } | |
| 44 | + | |
| 45 | + before(:each) do | |
| 46 | + delete :destroy, :app_id => problem.app.id, :err_id => problem.id, :id => comment.id.to_s | |
| 47 | + problem.reload | |
| 48 | + end | |
| 49 | + | |
| 50 | + it "should delete the comment" do | |
| 51 | + problem.comments.detect{|c| c.id.to_s == comment.id }.should == nil | |
| 52 | + end | |
| 53 | + | |
| 54 | + it "should redirect to problem page" do | |
| 55 | + response.should redirect_to( app_err_path(problem.app, problem) ) | |
| 56 | + end | |
| 57 | + end | |
| 58 | + end | |
| 59 | +end | ... | ... |
spec/controllers/errs_controller_spec.rb
| ... | ... | @@ -372,60 +372,6 @@ describe ErrsController do |
| 372 | 372 | end |
| 373 | 373 | end |
| 374 | 374 | |
| 375 | - | |
| 376 | - describe "POST /apps/:app_id/errs/:id/create_comment" do | |
| 377 | - render_views | |
| 378 | - | |
| 379 | - before(:each) do | |
| 380 | - sign_in Fabricate(:admin) | |
| 381 | - end | |
| 382 | - | |
| 383 | - context "successful comment creation" do | |
| 384 | - let(:problem) { Fabricate(:problem) } | |
| 385 | - let(:user) { Fabricate(:user) } | |
| 386 | - | |
| 387 | - before(:each) do | |
| 388 | - post :create_comment, :app_id => problem.app.id, :id => problem.id, | |
| 389 | - :comment => { :body => "One test comment", :user_id => user.id } | |
| 390 | - problem.reload | |
| 391 | - end | |
| 392 | - | |
| 393 | - it "should create the comment" do | |
| 394 | - problem.comments.size.should == 1 | |
| 395 | - end | |
| 396 | - | |
| 397 | - it "should redirect to problem page" do | |
| 398 | - response.should redirect_to( app_err_path(problem.app, problem) ) | |
| 399 | - end | |
| 400 | - end | |
| 401 | - end | |
| 402 | - | |
| 403 | - describe "DELETE /apps/:app_id/errs/:id/destroy_comment" do | |
| 404 | - render_views | |
| 405 | - | |
| 406 | - before(:each) do | |
| 407 | - sign_in Fabricate(:admin) | |
| 408 | - end | |
| 409 | - | |
| 410 | - context "successful comment deletion" do | |
| 411 | - let(:problem) { Fabricate(:problem_with_comments) } | |
| 412 | - let(:comment) { problem.reload.comments.first } | |
| 413 | - | |
| 414 | - before(:each) do | |
| 415 | - delete :destroy_comment, :app_id => problem.app.id, :id => problem.id, :comment_id => comment.id.to_s | |
| 416 | - problem.reload | |
| 417 | - end | |
| 418 | - | |
| 419 | - it "should delete the comment" do | |
| 420 | - problem.comments.detect{|c| c.id.to_s == comment.id }.should == nil | |
| 421 | - end | |
| 422 | - | |
| 423 | - it "should redirect to problem page" do | |
| 424 | - response.should redirect_to( app_err_path(problem.app, problem) ) | |
| 425 | - end | |
| 426 | - end | |
| 427 | - end | |
| 428 | - | |
| 429 | 375 | describe "Bulk Actions" do |
| 430 | 376 | before(:each) do |
| 431 | 377 | sign_in Fabricate(:admin) | ... | ... |