From c4549e910b79338c5e2df88e959fdd0127b35d45 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Sat, 6 Aug 2011 16:00:15 +0800 Subject: [PATCH] Added simple comments form to errors. (including tests) --- app/controllers/errs_controller.rb | 26 ++++++++++++++++++++++++++ app/models/comment.rb | 13 +++++++++++++ app/models/err.rb | 2 ++ app/views/errs/show.html.haml | 18 ++++++++++++++++++ app/views/layouts/application.html.haml | 4 ++++ config/routes.rb | 3 +++ public/stylesheets/application.css | 40 +++++++++++++++++++++++++++++++++++++--- spec/controllers/errs_controller_spec.rb | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ spec/factories/comment_factories.rb | 5 +++++ spec/factories/err_factories.rb | 8 +++++++- spec/models/comment_spec.rb | 12 ++++++++++++ spec/views/errs/show.html.haml_spec.rb | 13 ++++++++----- 12 files changed, 191 insertions(+), 9 deletions(-) create mode 100644 app/models/comment.rb create mode 100644 spec/factories/comment_factories.rb create mode 100644 spec/models/comment_spec.rb diff --git a/app/controllers/errs_controller.rb b/app/controllers/errs_controller.rb index 849d649..5a3899c 100644 --- a/app/controllers/errs_controller.rb +++ b/app/controllers/errs_controller.rb @@ -27,6 +27,7 @@ class ErrsController < ApplicationController page = 1 if page.to_i.zero? @notices = @err.notices.ordered.paginate(:page => page, :per_page => 1) @notice = @notices.first + @comment = Comment.new end def create_issue @@ -62,6 +63,30 @@ 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? + @err.comments << @comment + @err.save + flash[:success] = "Comment saved!" + else + flash[:error] = "I'm sorry, your comment was blank! Try again?" + end + redirect_to app_err_path(@app, @err) + 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, @err) + end + + protected def find_app @@ -83,3 +108,4 @@ class ErrsController < ApplicationController end end + diff --git a/app/models/comment.rb b/app/models/comment.rb new file mode 100644 index 0000000..649de1d --- /dev/null +++ b/app/models/comment.rb @@ -0,0 +1,13 @@ +class Comment + include Mongoid::Document + include Mongoid::Timestamps + + field :body, :type => String + index :user_id + + belongs_to :err + belongs_to :user + + validates_presence_of :body +end + diff --git a/app/models/err.rb b/app/models/err.rb index badadce..98473e0 100644 --- a/app/models/err.rb +++ b/app/models/err.rb @@ -18,6 +18,7 @@ class Err belongs_to :app has_many :notices + has_many :comments, :inverse_of => :err, :dependent => :destroy validates_presence_of :klass, :environment @@ -51,3 +52,4 @@ class Err end end + diff --git a/app/views/errs/show.html.haml b/app/views/errs/show.html.haml index 81f0ca3..dde396f 100644 --- a/app/views/errs/show.html.haml +++ b/app/views/errs/show.html.haml @@ -19,6 +19,23 @@ = link_to 'unlink issue', unlink_issue_app_err_path(@app, @err), :method => :delete, :confirm => "Unlink err issues?", :class => "unlink-issue" - if @err.unresolved? %span= link_to 'resolve', resolve_app_err_path(@app, @err), :method => :put, :confirm => err_confirm, :class => 'resolve' +- content_for :comments do + %h3 Comments on this Err + - @err.comments.each do |comment| + .window + %table.comment + %tr + %th + %span= link_to '✘'.html_safe, destroy_comment_app_err_path(@app, @err) << "?comment_id=#{comment.id}", :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 + + = form_for @comment, :url => create_comment_app_err_path(@app, @err) do |comment_form| + %p Add a comment + = comment_form.text_area :body, :style => "width: 420px; height: 80px;" + = comment_form.submit "Save Comment" %h4= @notice.try(:message) @@ -53,3 +70,4 @@ viewing occurrence #{@notices.current_page} of #{@notices.total_pages} #session %h3 Session = render 'notices/session', :notice => @notice + diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 416b2fb..287ddaa 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -28,5 +28,9 @@ #content = render :partial => 'shared/flash_messages' = yield + - if content_for?(:comments) + #content-comments + = yield :comments #footer= "Powered by #{link_to 'Errbit', 'http://github.com/jdpace/errbit', :target => '_blank'}: the open source error catcher.".html_safe = yield :scripts + diff --git a/config/routes.rb b/config/routes.rb index 6cc4244..c2ae40e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,6 +22,8 @@ Errbit::Application.routes.draw do put :resolve post :create_issue delete :unlink_issue + post :create_comment + delete :destroy_comment end end @@ -33,3 +35,4 @@ Errbit::Application.routes.draw do root :to => 'apps#index' end + diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index b1e529f..f11fd9c 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -139,14 +139,17 @@ a.action { float: right; font-size: 0.9em;} border: 1px solid #C6C6C6; } -/* Content Title */ -#content-title { +/* Content Title and Comments */ +#content-title, #content-comments { padding: 30px 20px; border-top: 1px solid #FFF; border-bottom: 1px solid #FFF; background-color: #e2e2e2; } -#content-title h1 { +#content-comments { + background-color: #ffffff; +} +#content-title h1, #content-comments h3 { padding: 0; margin: 0; width: 85%; border: none; @@ -154,6 +157,11 @@ a.action { float: right; font-size: 0.9em;} font-size: 2em; line-height: 1em; font-weight: bold; font-family: arial, sans-serif; word-wrap: break-word; } +#content-comments h3 { + font-size: 1.5em; + margin-bottom: 14px; +} + #content-title .meta { font-size: 0.9em; color: #787878; } /* Action Bar */ @@ -611,6 +619,7 @@ table.tally th.value { text-transform:none; } + /* Resolve Errs */ #action-bar a.resolve { background: transparent url(images/icons/thumbs-up.png) 6px 5px no-repeat; @@ -695,3 +704,28 @@ span.click_span { display: none; } +/* Comments */ +#content-comments form p { + margin: 30px 0 0 0; + text-transform: uppercase; +} +table.comment tbody th { + text-transform: none; + font-weight: normal; + height: 20px; + line-height: 0.5em; +} +table.comment tbody td { + background-color: #F9F9F9; +} +#content-comments a.destroy-comment { + color: #EE0000; + margin-right: 5px; +} +#content-comments a.destroy-comment:hover { + text-decoration: none; +} +#content-comments #comment_submit { + margin-top: 15px; +} + diff --git a/spec/controllers/errs_controller_spec.rb b/spec/controllers/errs_controller_spec.rb index 545c5ff..324669a 100644 --- a/spec/controllers/errs_controller_spec.rb +++ b/spec/controllers/errs_controller_spec.rb @@ -437,4 +437,60 @@ describe ErrsController do end end end + + + describe "POST /apps/:app_id/errs/:id/create_comment" do + render_views + + before(:each) do + sign_in Factory(:admin) + end + + context "successful comment creation" do + let(:err) { Factory(:err) } + let(:user) { Factory(:user) } + + before(:each) do + post :create_comment, :app_id => err.app.id, :id => err.id, + :comment => { :body => "One test comment", :user_id => user.id } + err.reload + end + + it "should create the comment" do + err.comments.size.should == 1 + end + + it "should redirect to err page" do + response.should redirect_to( app_err_path(err.app, err) ) + end + end + end + + describe "DELETE /apps/:app_id/errs/:id/destroy_comment" do + render_views + + before(:each) do + sign_in Factory(:admin) + end + + context "successful comment deletion" do + let(:err) { Factory :err_with_comments } + let(:comment) { err.comments.first } + + before(:each) do + delete :destroy_comment, :app_id => err.app.id, :id => err.id, :comment_id => comment.id + err.reload + end + + it "should delete the comment" do + err.comments.detect{|c| c.id.to_s == comment.id }.should == nil + end + + it "should redirect to err page" do + response.should redirect_to( app_err_path(err.app, err) ) + end + end + end + end + diff --git a/spec/factories/comment_factories.rb b/spec/factories/comment_factories.rb new file mode 100644 index 0000000..1bee372 --- /dev/null +++ b/spec/factories/comment_factories.rb @@ -0,0 +1,5 @@ +Factory.define :comment do |c| + c.user {|u| u.association :user} + c.body 'Test comment' +end + diff --git a/spec/factories/err_factories.rb b/spec/factories/err_factories.rb index e353b56..9f7afd4 100644 --- a/spec/factories/err_factories.rb +++ b/spec/factories/err_factories.rb @@ -4,6 +4,11 @@ Factory.define :err do |e| e.component 'foo' e.action 'bar' e.environment 'production' + e.comments [] +end + +Factory.define(:err_with_comments, :parent => :err) do |ec| + ec.comments { (1..3).map{Factory(:comment)} } end Factory.define :notice do |n| @@ -22,4 +27,5 @@ def random_backtrace 'method' => ActiveSupport.methods.shuffle.first }} backtrace -end \ No newline at end of file +end + diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb new file mode 100644 index 0000000..0103204 --- /dev/null +++ b/spec/models/comment_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe Comment do + context 'validations' do + it 'should require a body' do + comment = Factory.build(:comment, :body => nil) + comment.should_not be_valid + comment.errors[:body].should include("can't be blank") + end + end +end + diff --git a/spec/views/errs/show.html.haml_spec.rb b/spec/views/errs/show.html.haml_spec.rb index 51cfee7..02a27b2 100644 --- a/spec/views/errs/show.html.haml_spec.rb +++ b/spec/views/errs/show.html.haml_spec.rb @@ -1,9 +1,11 @@ require 'spec_helper' -describe "errs/show.html.erb" do - before do +describe "errs/show.html.erb" do + before do err = Factory(:err) + comment = Factory(:comment) assign :err, err + assign :comment, comment assign :app, err.app assign :notices, err.notices.ordered.paginate(:page => 1, :per_page => 1) assign :notice, err.notices.first @@ -12,7 +14,7 @@ describe "errs/show.html.erb" do describe "content_for :action_bar" do it "should confirm the 'resolve' link by default" do - render + render action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar]) resolve_link = action_bar.match(/()/)[0] resolve_link.should =~ /data-confirm="Seriously\?"/ @@ -20,7 +22,7 @@ describe "errs/show.html.erb" do it "should confirm the 'resolve' link if configuration is unset" do Errbit::Config.stub(:confirm_resolve_err).and_return(nil) - render + render action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar]) resolve_link = action_bar.match(/()/)[0] resolve_link.should =~ /data-confirm="Seriously\?"/ @@ -28,7 +30,7 @@ describe "errs/show.html.erb" do it "should not confirm the 'resolve' link if configured not to" do Errbit::Config.stub(:confirm_resolve_err).and_return(false) - render + render action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar]) resolve_link = action_bar.match(/()/)[0] resolve_link.should_not =~ /data-confirm=/ @@ -37,3 +39,4 @@ describe "errs/show.html.erb" do end end + -- libgit2 0.21.2