Commit e79cae05fe7949cbadef300e1d3f09cd2ba4f914

Authored by Nathan Broadbent
2 parents 75adfadc ba841cd1
Exists in master and in 1 other branch production

Merge branch 'comments_on_errors'

app/controllers/errs_controller.rb
@@ -27,6 +27,7 @@ class ErrsController < ApplicationController @@ -27,6 +27,7 @@ class ErrsController < ApplicationController
27 page = 1 if page.to_i.zero? 27 page = 1 if page.to_i.zero?
28 @notices = @err.notices.ordered.paginate(:page => page, :per_page => 1) 28 @notices = @err.notices.ordered.paginate(:page => page, :per_page => 1)
29 @notice = @notices.first 29 @notice = @notices.first
  30 + @comment = Comment.new
30 end 31 end
31 32
32 def create_issue 33 def create_issue
@@ -62,6 +63,30 @@ class ErrsController < ApplicationController @@ -62,6 +63,30 @@ class ErrsController < ApplicationController
62 redirect_to app_path(@app) 63 redirect_to app_path(@app)
63 end 64 end
64 65
  66 +
  67 + def create_comment
  68 + @comment = Comment.new(params[:comment].merge(:user_id => current_user.id))
  69 + if @comment.valid?
  70 + @err.comments << @comment
  71 + @err.save
  72 + flash[:success] = "Comment saved!"
  73 + else
  74 + flash[:error] = "I'm sorry, your comment was blank! Try again?"
  75 + end
  76 + redirect_to app_err_path(@app, @err)
  77 + end
  78 +
  79 + def destroy_comment
  80 + @comment = Comment.find(params[:comment_id])
  81 + if @comment.destroy
  82 + flash[:success] = "Comment deleted!"
  83 + else
  84 + flash[:error] = "Sorry, I couldn't delete your comment for some reason. I hope you don't have any sensitive information in there!"
  85 + end
  86 + redirect_to app_err_path(@app, @err)
  87 + end
  88 +
  89 +
65 protected 90 protected
66 91
67 def find_app 92 def find_app
@@ -83,3 +108,4 @@ class ErrsController &lt; ApplicationController @@ -83,3 +108,4 @@ class ErrsController &lt; ApplicationController
83 end 108 end
84 109
85 end 110 end
  111 +
app/helpers/application_helper.rb
1 module ApplicationHelper 1 module ApplicationHelper
2 -  
3 - 2 +
  3 +
4 def lighthouse_tracker? object 4 def lighthouse_tracker? object
5 object.issue_tracker_type == "lighthouseapp" 5 object.issue_tracker_type == "lighthouseapp"
6 end 6 end
7 - 7 +
8 def user_agent_graph(error) 8 def user_agent_graph(error)
9 tallies = tally(error.notices) {|notice| pretty_user_agent(notice.user_agent)} 9 tallies = tally(error.notices) {|notice| pretty_user_agent(notice.user_agent)}
10 create_percentage_table(tallies, :total => error.notices.count) 10 create_percentage_table(tallies, :total => error.notices.count)
11 end 11 end
12 - 12 +
13 def pretty_user_agent(user_agent) 13 def pretty_user_agent(user_agent)
14 (user_agent.nil? || user_agent.none?) ? "N/A" : "#{user_agent.browser} #{user_agent.version}" 14 (user_agent.nil? || user_agent.none?) ? "N/A" : "#{user_agent.browser} #{user_agent.version}"
15 end 15 end
16 - 16 +
17 def tally(collection, &block) 17 def tally(collection, &block)
18 collection.inject({}) do |tallies, item| 18 collection.inject({}) do |tallies, item|
19 value = yield item 19 value = yield item
@@ -21,7 +21,7 @@ module ApplicationHelper @@ -21,7 +21,7 @@ module ApplicationHelper
21 tallies 21 tallies
22 end 22 end
23 end 23 end
24 - 24 +
25 def create_percentage_table(tallies, options={}) 25 def create_percentage_table(tallies, options={})
26 total = (options[:total] || total_from_tallies(tallies)) 26 total = (options[:total] || total_from_tallies(tallies))
27 percent = 100.0 / total.to_f 27 percent = 100.0 / total.to_f
@@ -29,12 +29,16 @@ module ApplicationHelper @@ -29,12 +29,16 @@ module ApplicationHelper
29 .sort {|a, b| a[0] <=> b[0]} 29 .sort {|a, b| a[0] <=> b[0]}
30 render :partial => "errs/tally_table", :locals => {:rows => rows} 30 render :partial => "errs/tally_table", :locals => {:rows => rows}
31 end 31 end
32 - 32 +
33 def total_from_tallies(tallies) 33 def total_from_tallies(tallies)
34 tallies.values.inject(0) {|sum, n| sum + n} 34 tallies.values.inject(0) {|sum, n| sum + n}
35 end 35 end
36 private :total_from_tallies 36 private :total_from_tallies
37 - 37 +
  38 + def no_tracker? object
  39 + object.issue_tracker_type == "none"
  40 + end
  41 +
38 def redmine_tracker? object 42 def redmine_tracker? object
39 object.issue_tracker_type == "redmine" 43 object.issue_tracker_type == "redmine"
40 end 44 end
@@ -47,3 +51,4 @@ module ApplicationHelper @@ -47,3 +51,4 @@ module ApplicationHelper
47 object.issue_tracker_type == 'fogbugz' 51 object.issue_tracker_type == 'fogbugz'
48 end 52 end
49 end 53 end
  54 +
app/models/comment.rb 0 → 100644
@@ -0,0 +1,13 @@ @@ -0,0 +1,13 @@
  1 +class Comment
  2 + include Mongoid::Document
  3 + include Mongoid::Timestamps
  4 +
  5 + field :body, :type => String
  6 + index :user_id
  7 +
  8 + belongs_to :err
  9 + belongs_to :user
  10 +
  11 + validates_presence_of :body
  12 +end
  13 +
app/models/err.rb
@@ -18,6 +18,7 @@ class Err @@ -18,6 +18,7 @@ class Err
18 18
19 belongs_to :app 19 belongs_to :app
20 has_many :notices 20 has_many :notices
  21 + has_many :comments, :inverse_of => :err, :dependent => :destroy
21 22
22 validates_presence_of :klass, :environment 23 validates_presence_of :klass, :environment
23 24
@@ -51,3 +52,4 @@ class Err @@ -51,3 +52,4 @@ class Err
51 end 52 end
52 53
53 end 54 end
  55 +
app/views/apps/_fields.html.haml
@@ -53,7 +53,7 @@ @@ -53,7 +53,7 @@
53 %div.issue_tracker.nested 53 %div.issue_tracker.nested
54 %div.choose 54 %div.choose
55 = w.radio_button :issue_tracker_type, :none 55 = w.radio_button :issue_tracker_type, :none
56 - = label_tag :issue_tracker_type_lighthouseapp, '(None)', :for => label_for_attr(w, 'issue_tracker_type_none') 56 + = label_tag :issue_tracker_type_none, '(None)', :for => label_for_attr(w, 'issue_tracker_type_none')
57 = w.radio_button :issue_tracker_type, :lighthouseapp 57 = w.radio_button :issue_tracker_type, :lighthouseapp
58 = label_tag :issue_tracker_type_lighthouseapp, 'Lighthouse', :for => label_for_attr(w, 'issue_tracker_type_lighthouseapp') 58 = label_tag :issue_tracker_type_lighthouseapp, 'Lighthouse', :for => label_for_attr(w, 'issue_tracker_type_lighthouseapp')
59 = w.radio_button :issue_tracker_type, :redmine 59 = w.radio_button :issue_tracker_type, :redmine
@@ -62,6 +62,8 @@ @@ -62,6 +62,8 @@
62 = label_tag :issue_tracker_type_pivotal, 'Pivotal Tracker', :for => label_for_attr(w, 'issue_tracker_type_pivotal') 62 = label_tag :issue_tracker_type_pivotal, 'Pivotal Tracker', :for => label_for_attr(w, 'issue_tracker_type_pivotal')
63 = w.radio_button :issue_tracker_type, :fogbugz 63 = w.radio_button :issue_tracker_type, :fogbugz
64 = label_tag :issue_tracker_type_fogbugz, 'FogBugz', :for => label_for_attr(w, 'issue_tracker_type_fogbugz') 64 = label_tag :issue_tracker_type_fogbugz, 'FogBugz', :for => label_for_attr(w, 'issue_tracker_type_fogbugz')
  65 + %div.tracker_params.none{:class => no_tracker?(w.object) ? 'chosen' : nil}
  66 + %p When no issue tracker has been configured, you will be able to leave comments on errors.
65 %div.tracker_params.lighthouseapp{:class => lighthouse_tracker?(w.object) ? 'chosen' : nil} 67 %div.tracker_params.lighthouseapp{:class => lighthouse_tracker?(w.object) ? 'chosen' : nil}
66 = w.label :account, "Account" 68 = w.label :account, "Account"
67 = w.text_field :account, :placeholder => "abc from abc.lighthouseapp.com" 69 = w.text_field :account, :placeholder => "abc from abc.lighthouseapp.com"
app/views/errs/show.html.haml
@@ -20,6 +20,25 @@ @@ -20,6 +20,25 @@
20 - if @err.unresolved? 20 - if @err.unresolved?
21 %span= link_to 'resolve', resolve_app_err_path(@app, @err), :method => :put, :confirm => err_confirm, :class => 'resolve' 21 %span= link_to 'resolve', resolve_app_err_path(@app, @err), :method => :put, :confirm => err_confirm, :class => 'resolve'
22 22
  23 +- if !@app.issue_tracker_configured? || @err.comments.any?
  24 + - content_for :comments do
  25 + %h3 Comments on this Err
  26 + - @err.comments.each do |comment|
  27 + .window
  28 + %table.comment
  29 + %tr
  30 + %th
  31 + %span= link_to '&#10008;'.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"
  32 + = time_ago_in_words(comment.created_at, true) << " ago by "
  33 + = link_to comment.user.email, user_path(comment.user)
  34 + %tr
  35 + %td= comment.body.gsub("\n", "<br>").html_safe
  36 + - unless @app.issue_tracker_configured?
  37 + = form_for @comment, :url => create_comment_app_err_path(@app, @err) do |comment_form|
  38 + %p Add a comment
  39 + = comment_form.text_area :body, :style => "width: 420px; height: 80px;"
  40 + = comment_form.submit "Save Comment"
  41 +
23 %h4= @notice.try(:message) 42 %h4= @notice.try(:message)
24 43
25 = will_paginate @notices, :param_name => :notice, :page_links => false, :class => 'notice-pagination' 44 = will_paginate @notices, :param_name => :notice, :page_links => false, :class => 'notice-pagination'
@@ -53,3 +72,4 @@ viewing occurrence #{@notices.current_page} of #{@notices.total_pages} @@ -53,3 +72,4 @@ viewing occurrence #{@notices.current_page} of #{@notices.total_pages}
53 #session 72 #session
54 %h3 Session 73 %h3 Session
55 = render 'notices/session', :notice => @notice 74 = render 'notices/session', :notice => @notice
  75 +
app/views/layouts/application.html.haml
@@ -28,5 +28,9 @@ @@ -28,5 +28,9 @@
28 #content 28 #content
29 = render :partial => 'shared/flash_messages' 29 = render :partial => 'shared/flash_messages'
30 = yield 30 = yield
  31 + - if content_for?(:comments)
  32 + #content-comments
  33 + = yield :comments
31 #footer= "Powered by #{link_to 'Errbit', 'http://github.com/jdpace/errbit', :target => '_blank'}: the open source error catcher.".html_safe 34 #footer= "Powered by #{link_to 'Errbit', 'http://github.com/jdpace/errbit', :target => '_blank'}: the open source error catcher.".html_safe
32 = yield :scripts 35 = yield :scripts
  36 +
config/routes.rb
@@ -22,6 +22,8 @@ Errbit::Application.routes.draw do @@ -22,6 +22,8 @@ Errbit::Application.routes.draw do
22 put :resolve 22 put :resolve
23 post :create_issue 23 post :create_issue
24 delete :unlink_issue 24 delete :unlink_issue
  25 + post :create_comment
  26 + delete :destroy_comment
25 end 27 end
26 end 28 end
27 29
@@ -33,3 +35,4 @@ Errbit::Application.routes.draw do @@ -33,3 +35,4 @@ Errbit::Application.routes.draw do
33 root :to => 'apps#index' 35 root :to => 'apps#index'
34 36
35 end 37 end
  38 +
public/stylesheets/application.css
@@ -139,14 +139,17 @@ a.action { float: right; font-size: 0.9em;} @@ -139,14 +139,17 @@ a.action { float: right; font-size: 0.9em;}
139 border: 1px solid #C6C6C6; 139 border: 1px solid #C6C6C6;
140 } 140 }
141 141
142 -/* Content Title */  
143 -#content-title { 142 +/* Content Title and Comments */
  143 +#content-title, #content-comments {
144 padding: 30px 20px; 144 padding: 30px 20px;
145 border-top: 1px solid #FFF; 145 border-top: 1px solid #FFF;
146 border-bottom: 1px solid #FFF; 146 border-bottom: 1px solid #FFF;
147 background-color: #e2e2e2; 147 background-color: #e2e2e2;
148 } 148 }
149 -#content-title h1 { 149 +#content-comments {
  150 + background-color: #ffffff;
  151 +}
  152 +#content-title h1, #content-comments h3 {
150 padding: 0; margin: 0; 153 padding: 0; margin: 0;
151 width: 85%; 154 width: 85%;
152 border: none; 155 border: none;
@@ -154,6 +157,11 @@ a.action { float: right; font-size: 0.9em;} @@ -154,6 +157,11 @@ a.action { float: right; font-size: 0.9em;}
154 font-size: 2em; line-height: 1em; font-weight: bold; font-family: arial, sans-serif; 157 font-size: 2em; line-height: 1em; font-weight: bold; font-family: arial, sans-serif;
155 word-wrap: break-word; 158 word-wrap: break-word;
156 } 159 }
  160 +#content-comments h3 {
  161 + font-size: 1.5em;
  162 + margin-bottom: 14px;
  163 +}
  164 +
157 #content-title .meta { font-size: 0.9em; color: #787878; } 165 #content-title .meta { font-size: 0.9em; color: #787878; }
158 166
159 /* Action Bar */ 167 /* Action Bar */
@@ -610,6 +618,7 @@ table.tally th.value { @@ -610,6 +618,7 @@ table.tally th.value {
610 text-transform:none; 618 text-transform:none;
611 } 619 }
612 620
  621 +
613 /* Resolve Errs */ 622 /* Resolve Errs */
614 #action-bar a.resolve { 623 #action-bar a.resolve {
615 background: transparent url(images/icons/thumbs-up.png) 6px 5px no-repeat; 624 background: transparent url(images/icons/thumbs-up.png) 6px 5px no-repeat;
@@ -694,3 +703,28 @@ span.click_span { @@ -694,3 +703,28 @@ span.click_span {
694 display: none; 703 display: none;
695 } 704 }
696 705
  706 +/* Comments */
  707 +#content-comments form p {
  708 + margin: 30px 0 0 0;
  709 + text-transform: uppercase;
  710 +}
  711 +table.comment tbody th {
  712 + text-transform: none;
  713 + font-weight: normal;
  714 + height: 20px;
  715 + line-height: 0.5em;
  716 +}
  717 +table.comment tbody td {
  718 + background-color: #F9F9F9;
  719 +}
  720 +#content-comments a.destroy-comment {
  721 + color: #EE0000;
  722 + margin-right: 5px;
  723 +}
  724 +#content-comments a.destroy-comment:hover {
  725 + text-decoration: none;
  726 +}
  727 +#content-comments #comment_submit {
  728 + margin-top: 15px;
  729 +}
  730 +
spec/controllers/errs_controller_spec.rb
@@ -437,4 +437,60 @@ describe ErrsController do @@ -437,4 +437,60 @@ describe ErrsController do
437 end 437 end
438 end 438 end
439 end 439 end
  440 +
  441 +
  442 + describe "POST /apps/:app_id/errs/:id/create_comment" do
  443 + render_views
  444 +
  445 + before(:each) do
  446 + sign_in Factory(:admin)
  447 + end
  448 +
  449 + context "successful comment creation" do
  450 + let(:err) { Factory(:err) }
  451 + let(:user) { Factory(:user) }
  452 +
  453 + before(:each) do
  454 + post :create_comment, :app_id => err.app.id, :id => err.id,
  455 + :comment => { :body => "One test comment", :user_id => user.id }
  456 + err.reload
  457 + end
  458 +
  459 + it "should create the comment" do
  460 + err.comments.size.should == 1
  461 + end
  462 +
  463 + it "should redirect to err page" do
  464 + response.should redirect_to( app_err_path(err.app, err) )
  465 + end
  466 + end
  467 + end
  468 +
  469 + describe "DELETE /apps/:app_id/errs/:id/destroy_comment" do
  470 + render_views
  471 +
  472 + before(:each) do
  473 + sign_in Factory(:admin)
  474 + end
  475 +
  476 + context "successful comment deletion" do
  477 + let(:err) { Factory :err_with_comments }
  478 + let(:comment) { err.comments.first }
  479 +
  480 + before(:each) do
  481 + delete :destroy_comment, :app_id => err.app.id, :id => err.id, :comment_id => comment.id
  482 + err.reload
  483 + end
  484 +
  485 + it "should delete the comment" do
  486 + err.comments.detect{|c| c.id.to_s == comment.id }.should == nil
  487 + end
  488 +
  489 + it "should redirect to err page" do
  490 + response.should redirect_to( app_err_path(err.app, err) )
  491 + end
  492 + end
  493 + end
  494 +
440 end 495 end
  496 +
spec/factories/comment_factories.rb 0 → 100644
@@ -0,0 +1,5 @@ @@ -0,0 +1,5 @@
  1 +Factory.define :comment do |c|
  2 + c.user {|u| u.association :user}
  3 + c.body 'Test comment'
  4 +end
  5 +
spec/factories/err_factories.rb
@@ -4,6 +4,11 @@ Factory.define :err do |e| @@ -4,6 +4,11 @@ Factory.define :err do |e|
4 e.component 'foo' 4 e.component 'foo'
5 e.action 'bar' 5 e.action 'bar'
6 e.environment 'production' 6 e.environment 'production'
  7 + e.comments []
  8 +end
  9 +
  10 +Factory.define(:err_with_comments, :parent => :err) do |ec|
  11 + ec.comments { (1..3).map{Factory(:comment)} }
7 end 12 end
8 13
9 Factory.define :notice do |n| 14 Factory.define :notice do |n|
@@ -22,4 +27,5 @@ def random_backtrace @@ -22,4 +27,5 @@ def random_backtrace
22 'method' => ActiveSupport.methods.shuffle.first 27 'method' => ActiveSupport.methods.shuffle.first
23 }} 28 }}
24 backtrace 29 backtrace
25 -end  
26 \ No newline at end of file 30 \ No newline at end of file
  31 +end
  32 +
spec/models/comment_spec.rb 0 → 100644
@@ -0,0 +1,12 @@ @@ -0,0 +1,12 @@
  1 +require 'spec_helper'
  2 +
  3 +describe Comment do
  4 + context 'validations' do
  5 + it 'should require a body' do
  6 + comment = Factory.build(:comment, :body => nil)
  7 + comment.should_not be_valid
  8 + comment.errors[:body].should include("can't be blank")
  9 + end
  10 + end
  11 +end
  12 +
spec/views/errs/show.html.haml_spec.rb
1 require 'spec_helper' 1 require 'spec_helper'
2 2
3 -describe "errs/show.html.erb" do  
4 - before do 3 +describe "errs/show.html.erb" do
  4 + before do
5 err = Factory(:err) 5 err = Factory(:err)
  6 + comment = Factory(:comment)
6 assign :err, err 7 assign :err, err
  8 + assign :comment, comment
7 assign :app, err.app 9 assign :app, err.app
8 assign :notices, err.notices.ordered.paginate(:page => 1, :per_page => 1) 10 assign :notices, err.notices.ordered.paginate(:page => 1, :per_page => 1)
9 assign :notice, err.notices.first 11 assign :notice, err.notices.first
@@ -12,7 +14,7 @@ describe &quot;errs/show.html.erb&quot; do @@ -12,7 +14,7 @@ describe &quot;errs/show.html.erb&quot; do
12 describe "content_for :action_bar" do 14 describe "content_for :action_bar" do
13 15
14 it "should confirm the 'resolve' link by default" do 16 it "should confirm the 'resolve' link by default" do
15 - render 17 + render
16 action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar]) 18 action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar])
17 resolve_link = action_bar.match(/(<a href.*?(class="resolve").*?>)/)[0] 19 resolve_link = action_bar.match(/(<a href.*?(class="resolve").*?>)/)[0]
18 resolve_link.should =~ /data-confirm="Seriously\?"/ 20 resolve_link.should =~ /data-confirm="Seriously\?"/
@@ -20,7 +22,7 @@ describe &quot;errs/show.html.erb&quot; do @@ -20,7 +22,7 @@ describe &quot;errs/show.html.erb&quot; do
20 22
21 it "should confirm the 'resolve' link if configuration is unset" do 23 it "should confirm the 'resolve' link if configuration is unset" do
22 Errbit::Config.stub(:confirm_resolve_err).and_return(nil) 24 Errbit::Config.stub(:confirm_resolve_err).and_return(nil)
23 - render 25 + render
24 action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar]) 26 action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar])
25 resolve_link = action_bar.match(/(<a href.*?(class="resolve").*?>)/)[0] 27 resolve_link = action_bar.match(/(<a href.*?(class="resolve").*?>)/)[0]
26 resolve_link.should =~ /data-confirm="Seriously\?"/ 28 resolve_link.should =~ /data-confirm="Seriously\?"/
@@ -28,7 +30,7 @@ describe &quot;errs/show.html.erb&quot; do @@ -28,7 +30,7 @@ describe &quot;errs/show.html.erb&quot; do
28 30
29 it "should not confirm the 'resolve' link if configured not to" do 31 it "should not confirm the 'resolve' link if configured not to" do
30 Errbit::Config.stub(:confirm_resolve_err).and_return(false) 32 Errbit::Config.stub(:confirm_resolve_err).and_return(false)
31 - render 33 + render
32 action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar]) 34 action_bar = String.new(view.instance_variable_get(:@_content_for)[:action_bar])
33 resolve_link = action_bar.match(/(<a href.*?(class="resolve").*?>)/)[0] 35 resolve_link = action_bar.match(/(<a href.*?(class="resolve").*?>)/)[0]
34 resolve_link.should_not =~ /data-confirm=/ 36 resolve_link.should_not =~ /data-confirm=/
@@ -36,4 +38,38 @@ describe &quot;errs/show.html.erb&quot; do @@ -36,4 +38,38 @@ describe &quot;errs/show.html.erb&quot; do
36 38
37 end 39 end
38 40
  41 + describe "content_for :comments" do
  42 + it 'should display comments and new comment form when no issue tracker' do
  43 + err = Factory(:err_with_comments)
  44 + assign :err, err
  45 + assign :app, err.app
  46 + render
  47 + comments_section = String.new(view.instance_variable_get(:@_content_for)[:comments])
  48 + comments_section.should =~ /Test comment/
  49 + comments_section.should =~ /Add a comment/
  50 + end
  51 +
  52 + context "with issue tracker" do
  53 + def with_issue_tracker(err)
  54 + err.app.issue_tracker = IssueTracker.new :issue_tracker_type => "lighthouseapp", :project_id => "1234"
  55 + assign :err, err
  56 + assign :app, err.app
  57 + end
  58 +
  59 + it 'should not display the comments section' do
  60 + with_issue_tracker(Factory(:err))
  61 + render
  62 + view.instance_variable_get(:@_content_for)[:comments].should be_blank
  63 + end
  64 +
  65 + it 'should display existing comments' do
  66 + with_issue_tracker(Factory(:err_with_comments))
  67 + render
  68 + comments_section = String.new(view.instance_variable_get(:@_content_for)[:comments])
  69 + comments_section.should =~ /Test comment/
  70 + comments_section.should_not =~ /Add a comment/
  71 + end
  72 + end
  73 + end
39 end 74 end
  75 +