Commit 5155cbc247be6649bb553b3cf661b8f435aaabb3
1 parent
cddea1b5
Exists in
master
and in
1 other branch
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) | ... | ... |