Commit ba841cd1157b12e0cc2f62c3ae245cd7c05b100b
1 parent
c4549e91
Exists in
master
and in
1 other branch
Comments are only enabled if app has no issue tracker configured.
Showing
4 changed files
with
67 additions
and
25 deletions
Show diff stats
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/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
| @@ -19,23 +19,25 @@ | @@ -19,23 +19,25 @@ | ||
| 19 | = link_to 'unlink issue', unlink_issue_app_err_path(@app, @err), :method => :delete, :confirm => "Unlink err issues?", :class => "unlink-issue" | 19 | = link_to 'unlink issue', unlink_issue_app_err_path(@app, @err), :method => :delete, :confirm => "Unlink err issues?", :class => "unlink-issue" |
| 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 | -- content_for :comments do | ||
| 23 | - %h3 Comments on this Err | ||
| 24 | - - @err.comments.each do |comment| | ||
| 25 | - .window | ||
| 26 | - %table.comment | ||
| 27 | - %tr | ||
| 28 | - %th | ||
| 29 | - %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" | ||
| 30 | - = time_ago_in_words(comment.created_at, true) << " ago by " | ||
| 31 | - = link_to comment.user.email, user_path(comment.user) | ||
| 32 | - %tr | ||
| 33 | - %td= comment.body.gsub("\n", "<br>").html_safe | ||
| 34 | 22 | ||
| 35 | - = form_for @comment, :url => create_comment_app_err_path(@app, @err) do |comment_form| | ||
| 36 | - %p Add a comment | ||
| 37 | - = comment_form.text_area :body, :style => "width: 420px; height: 80px;" | ||
| 38 | - = comment_form.submit "Save Comment" | 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 '✘'.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" | ||
| 39 | 41 | ||
| 40 | %h4= @notice.try(:message) | 42 | %h4= @notice.try(:message) |
| 41 | 43 |
spec/views/errs/show.html.haml_spec.rb
| @@ -38,5 +38,38 @@ describe "errs/show.html.erb" do | @@ -38,5 +38,38 @@ describe "errs/show.html.erb" do | ||
| 38 | 38 | ||
| 39 | end | 39 | end |
| 40 | 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 | ||
| 41 | end | 74 | end |
| 42 | 75 |